-
Notifications
You must be signed in to change notification settings - Fork 33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Account for problems being fixed in Python 3.12 #110
Conversation
GHA no longer supports Python 2.7: actions/setup-python#672 While there are ways we could continue testing 2.7, this seems like a good time to let it go. Anyone using 2.7 can still use an older version of asttokens (i.e. the current one) and they should be fine. Newer versions of asttokens have generally been motivated by newer versions of Python anyway, as with this PR. |
Using astroid from git HEAD and this PR, we get this in Fedora:
|
Is it possible that this would in fact be a problem in astroid? |
@alexmojaki This is not a review of the fix you created but of the deprecation of Python 2.7. Don't forget to remove the python 2 and 2.7 classifiers and the dependency on six. Although, I think that should be done in a separate PR that gets merged before this one. I can give it a shot if you create an GH issue and assign it to me. |
Thanks, this is an important reminder.
I don't really see why continuing to depend on six is a critical problem that should block this PR (although this PR isn't actually ready to merge yet), even without Python 2 support.
I think we need to decide to actually drop Python 2 support, and that's up to @dsagal. But I've opened #111 for that discussion to continue there. |
asttokens/mark_tokens.py
Outdated
if sys.version_info >= (3, 12): | ||
last = first_token | ||
while True: | ||
if util.match_token(last, getattr(token, "FSTRING_START")): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add brief comments here, or maybe an example? Also curious why the syntax getattr(token, "FSTRING_START")
vs token.FSTRING_START
? Is that mainly for editor/linting convenience?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, mypy doesn't currently accept token.FSTRING_START
: https://github.com/gristlabs/asttokens/actions/runs/6036177479/job/16377962926
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amazingly, it only complains about it in 3.12. My guess is that earlier versions respect the sys.version_info >= (3, 12)
. But they still complain about type: ignore
if I use that, claiming it's unused: https://github.com/gristlabs/asttokens/actions/runs/6036208109/job/16378046034
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
Closes #109
Seems that several things have significantly improved in 3.12!