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

emmatyping
Copy link
Collaborator

@emmatyping emmatyping 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).

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

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

@emmatyping emmatyping 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.

@emmatyping
Copy link
Collaborator Author

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

@emmatyping emmatyping closed this Mar 7, 2018
@emmatyping emmatyping 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