-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Suggest using a newer Python version if possibly needed #13197
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
Conversation
This comment has been minimized.
This comment has been minimized.
Could the output mention the current interpreter? the original issue was on the playground using 3.10 with |
test-data/unit/check-newsyntax.test
Outdated
# flags: --python-version 3.99 | ||
this is what future python looks like public static void main String[] args await goto exit | ||
[out] | ||
main:2: error: invalid syntax; maybe you need to use a newer version of Python to run mypy? |
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.
I agree with @KotlinIsland, I think we need to show versions to users and explain what is going on with more details. Because it is confusing indeed.
main:2: error: invalid syntax; maybe you need to use a newer version of Python to run mypy? | |
main:2: error: invalid syntax; maybe you need to use a newer version of Python to run mypy? | |
python3.10 cannot parse syntax from python3.99 |
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.
What about we update the ast
module in older versions of python to be able to parse newer python?
Could also make it an installable package.
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.
I maintain too many ast
hacks to agree 🙂
Example: https://github.com/wemake-services/wemake-python-styleguide/tree/master/wemake_python_styleguide/compat
Probably using a better message is the best we can do in the long run.
Trying to parse future versions of python with older ones is way out of our main goal.
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.
Yeah, agreed that's a nice to have. But I don't have time to spend wiring up the test infra just to allow a version specific error message for this.
If this is an improvement over the status quo, let's merge, otherwise I'll close this and someone else can look into shipping it with a better error message :-)
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.
That's basically what typed-ast was, and it was a bit of a pain to maintain (#6545)
This comment has been minimized.
This comment has been minimized.
test-data/unit/check-newsyntax.test
Outdated
# flags: --python-version 3.99 | ||
this is what future python looks like public static void main String[] args await goto exit | ||
[out] | ||
main:2: error: invalid syntax; you likely need to run mypy using a newer version of Python. |
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.
A few nits:
- Drop the final dot, for consistency?
- What about only showing the target version in the error message, such as "... you likely need to run mypy using Python 3.99 or newer"? This should be easy to do, I think.
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.
Good point, fixed both
This comment has been minimized.
This comment has been minimized.
According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉 |
Fixes #12357