Skip to content
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

Add --python-executable and --no-infer-executable flags #4692

Closed
wants to merge 6 commits into from

Conversation

ethanhs
Copy link
Collaborator

@ethanhs ethanhs commented Mar 7, 2018

This implements a flag that allows users to point to a Python interpreter that mypy can use for type checking. --no-infer-executable is named as such because the old name (--no-site-packages) was rather misleading.

Fixes #965

Carried over from #4403, with some changes (renaming the flag, documentation).

@ethanhs ethanhs changed the title [WIP]Add --python-executable and --no-site-packages flags [WIP]Add --python-executable and --no-infer-executable flags Mar 7, 2018
@ethanhs ethanhs changed the title [WIP]Add --python-executable and --no-infer-executable flags Add --python-executable and --no-infer-executable flags Mar 7, 2018
@ethanhs
Copy link
Collaborator Author

ethanhs commented Mar 7, 2018

This should be ready for review now. I think the testing I have exercises the new flags well.

@ethanhs ethanhs mentioned this pull request Mar 7, 2018
8 tasks
will attempt to find a Python executable of the corresponding version. If
you'd like to disable this, see ``--no-infer-executable`` below.

- ``--no-infer-executable`` will disable searching for a usable Python
Copy link
Contributor

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 flag makes any sense without PEP561, and that --no-site-packages was a better name in that scenario, as it indicated the purpose of the executable

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, I think I might agree in principle with the first (but I split this out for ease of review mostly).

As for the flag name, I'm not so sure. If the executable is ever used for anything other than PEP 561, the --no-site-packages flag would be misleading. It is misleading now anyway, it doesn't actually disable searching, it sets options.python_executable to None, which happens to mean that searching for PEP 561 packages is not done.

Copy link
Contributor

@eric-wieser eric-wieser Mar 7, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it sets options.python_executable to None, which happens to mean that searching for PEP 561 packages is not done.

You've got this backwards. options.python_executable is None is an implementation-detail of how we represent "don't do pep561 searching", chosen over storing a redundant boolean alongside it. We could add a @property def do_pep561_searching to Options to make that more explicit, I suppose.

We shouldn't be exposing that detail in the command line arguments.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, I suppose from a user perspective it would be better to name it --no-site-packages.

@ethanhs
Copy link
Collaborator Author

ethanhs commented Mar 7, 2018

Closing as this should be part of the main PEP implementation.

@ethanhs ethanhs closed this Mar 7, 2018
@ethanhs ethanhs deleted the python-executable branch April 12, 2018 08:09
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.

2 participants