KEMBAR78
Use sys.executable for invoking mypy in runtests if it is Python 3 by emmatyping · Pull Request #5702 · python/mypy · GitHub
Skip to content

Conversation

@emmatyping
Copy link
Member

I recently installed Python 3.7, which means that the py launcher 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?

Copy link
Collaborator

@Michael0x2a Michael0x2a left a 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?)

@emmatyping
Copy link
Member Author

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.

Yes, that makes sense. Done.

@emmatyping emmatyping changed the title Use sys.executable on Windows for invoking mypy if it is Python 3 Use sys.executable for invoking mypy in runtests if it is Python 3 Oct 1, 2018
@gvanrossum
Copy link
Member

While you're at it, could you also add a mention of runtests.py to the toplevel README.md? Probably in the section on Tests.

Copy link
Member

@gvanrossum gvanrossum left a 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 >=?)

@emmatyping
Copy link
Member Author

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):
Copy link
Member

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.

Copy link
Member Author

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.

Copy link
Member

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.

@Michael0x2a
Copy link
Collaborator

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)

@emmatyping emmatyping merged commit c47511f into python:master Oct 1, 2018
@emmatyping emmatyping deleted the sys-exe-runtests branch October 1, 2018 21:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants