-
-
Notifications
You must be signed in to change notification settings - Fork 3k
Use sys.executable for invoking mypy in runtests if it is Python 3 #5702
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
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 don't think this should break anything for me.
One question though -- would it maybe be a good idea to just default to using sys.executable for both windows and linux/mac? For example, maybe somebody using Linux might want to try running this script using a version of Python other then whatever python3 is pointing to.
It'll make the output we're printing below messier though, so maybe it isn't worth it, not sure.
I'm ok with this landing either way though -- it looks like both py -3 and sys.executable do the correct thing for me, and I don't really have strong opinions about this script in general.
Semi-related: if you run into similar "py -3 isn't pointing to the thing I want" issues in the future, it looks like the launcher will let you configure which version is run via an environment variables (and maybe also a config file?)
Yes, that makes sense. Done. |
|
While you're at it, could you also add a mention of |
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.
OK, this looks reasonable. (Though was there really something wrong with Python 3.0.0? Or did you mean >=?)
I did, however now that I think about it, it should be >= 3.4.0 since that is the minimum mypy supports. |
| python_name = 'py -3' | ||
|
|
||
| # Use the Python provided to execute the script, or fall back to a sane default | ||
| if version_info >= (3, 4, 0): |
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.
As long as we're nit-picking here, I'd say if version_info[0] >= 3 -- this speaks to our intention which is that we're really just distinguishing between (supported versions of) Python 2 and 3.
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.
Perhaps you missed my comment here, but I think this is not the right thing to do. We want to select only a version of Python 3 that mypy supports.
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 did see that comment but wasn't sure how it mattered...
So the case where this matters is apparently one where runtests.py is explicitly invoked with an unsupported version of Python 3, e.g. if the user typed python3.3 runtests.py while the default python3 refers to 3.4 or higher. Right? I can dig that, though it's too bad that we can't import mypy.defaults (and please don't try to find a way to be able to import that -- I think it's important that this script is clearly independent from the mypy package).
So now I agree.
|
I think it would also be reasonable to have this script just terminate if somebody tries running it with an unsupported Python version. I think it's reasonable to expect anybody who's hacking on mypy to know that you need to run it using only Python 3 -- and if they're not aware, it's probably easier to correct that misconception sooner rather then later. (But like I said, I really don't have strong opinions about this script) |
I recently installed Python 3.7, which means that the
pylauncher will use it as default (even though I had installed it for testing...). I have Python 3.6 as the default for executing scripts, and the main development environment I use.This change will use the Python executable used to run the script, instead of the default Python 3 executable (via
py -3), if possible, or will fall back to the old behavior if the default to run a script is a Python 2 executable.@Michael0x2a hopefully this doesn't break anything for you?