Repository navigation
Allow setting python_executable from config file... - #5777
Conversation
|
I realized this isn't actually correct yet, so I'm not quite ready for review yet. |
|
It appears the new version of flake8 is very picky. The Travis failure is not related to these changes (its just flake8). I will submit a PR to fix this. |
|
Okay this should be ready for review. |
msullivan
left a comment
There was a problem hiding this comment.
This looks good.
It might be worth adding a test that actually does read the version from a config file
There are several tests for reading the version from the config file already (in |
|
I will merge this later today unless someone has suggestions for changes. (@msullivan not sure if you wanted to respond to my above comment). |
|
I am trying to come up with an end-to-end test for this, and it's not working. I found a pypi package ordered_set (aka ordered-set) that has stubs on pypi. I am installing these with: To prove that it works: This gives no errors. Then with mypy from master, I run: This error is expected (there's a bug in the stubs that only happens to manifest with Python 2). (Note that But with mypy from this branch I can't get it to work: I can get it to work by installing the stubs in the Python 3 venv I use to run mypy, or by explicitly specifying a Python executable: But that seems a step back. |
| assert options.python_version == sys.version_info[:2] | ||
| assert options.python_executable == sys.executable | ||
|
|
||
| # then test inferring version from executable |
There was a problem hiding this comment.
Is this testing anything any more? You don't infer the version from the executable any more in the code.
gvanrossum
left a comment
There was a problem hiding this comment.
(Unapproving since I can't get it to work as I expect end-to-end.)
|
Ah, yes I have it using the default Python executable. I will need to think this over a bit. I believe the issue is that we want to be able to use I suppose the "correct" thing to do would probably be to make |
|
Hm, it looks like the logic for the Python version could be simplified: just use the default
It looks like everything becomes easier that way. |
|
@ethanhs Any change to get this in for the 0.650 release? I think it would be a nice improvement. |
|
(If you want to push back on my suggestion to simplify the implementation and just merge the current version I could be okay with that too.) |
Decided I want the fix more badly than the simplified implementation.
|
Oh, sorry I seem to have missed your message from two weeks back. I will make another PR tomorrow with the simplification. |
... and let the
python_versionandpython_executablebe different, so that one can install stub packages into the Python used to run mypy.Fixes #5620.