Fix MyPy to work by default with Python 3.6+ code and Black with 3.8+ code #10750
Conversation
[ci skip-rust] [ci skip-build-wheels]
Eric-Arellano
force-pushed
the
mypy-version
branch
from
dba5e31
to
c78cba0
Compare
|
Gr. Our CI fails because I removed our internal setting, and now MyPy runs with Python 3.5. This complains that it doesn't understand f strings, even though typed-ast should allow this to work! Looks like I need more debugging with MyPy... |
I figured it out. MyPy tells typed-ast what Python version to use based on the https://github.com/python/mypy/blob/d7553fe181f91b6bbf100ca16db83f67690e669a/mypy/fastparse.py#L163 If unspecified, I think this defaults to the interpreter being used to run. I can fix this by adding -- We could tell users to set They can still set |
Also fix both Black and MyPy to respect the --interpreter-constraints option if it was configured. # Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
Eric-Arellano
changed the title
| import datetime | ||
|
|
||
| x = True | ||
| if y := x: | ||
| print("x is truthy and now assigned to y") | ||
|
|
||
|
|
||
| class Foo: | ||
| pass |
There was a problem hiding this comment.
This was a real pain to get MyPy to error. If I just had the walrus operator portion, MyPy was fine..I don't know why we need the import and class declaration. I spent so much time trying to figure out how to get MyPy to error that I went with it once I got it working.
| # * We must resolve third-party dependencies. This should use whatever the actual code's | ||
| # constraints are, as the constraints for the tool can be different than for the | ||
| # requirements. | ||
| # * The runner Pex should use the same constraints as the tool Pex. |
There was a problem hiding this comment.
This part is tomorrow's project..
| # https://mypy.readthedocs.io/en/stable/config_file.html#platform-configuration. We do | ||
| # not auto-configure this for simplicity and to avoid Pants magically setting values for | ||
| # users. |
There was a problem hiding this comment.
Lol to the simpler days where I thought this would work.
| py2 = is_python2( | ||
| python_setup.compatibilities_or_constraints( | ||
| target_with_origin.target.get(PythonInterpreterCompatibility).value | ||
| interpreter_constraints = PexInterpreterConstraints.create_from_compatibility_fields( |
There was a problem hiding this comment.
This refactor was convenient because we added similar new methods to PexInterpreterConstraints for Black and MyPy. Now they're located together.
# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
Problem
MyPy and Black both use typed-ast for parsing, just like we use it for dependency inference. Typed-ast understands Py27 and 34-37, but not Py38. For Py38, you instead must run with a Python 3.8 interpreter (which also understands 27 and 34-37).
This meant that our default Black and MyPy installations would break if encountering Py38 syntax like the walrus operator. You had to explicitly set
--black-interpreter-constrainsand--mypy-interpreter-constraints, which is not at all obvious.Solution
Detect if the user has constraints for 3.8+. If so, use the constraints
>=3.8for the tools. Otherwise, use the tool's default constraints.We check that the constraints do not include anything less than 3.8, such as 3.7. Why? If they need to support 3.7, then a user won't be using 3.8 syntax. We don't want to constraint to
>=3.8because this would mean that they must have 3.8 on their machine, even though 3.7 would do just fine.To facilitate this, we change
PexInterpreterConstraintsto store parsedRequirementobjects.Also fixes MyPy handing of Python 3.6 and Python 3.7
If the user leaves off
python_version, MyPy defaults to the version of the interpreter used to run. MyPy also tells the AST parser to target this specificpython_version. So, if you're running with our minimum requirement of Python 3.5, it will fail to parse any Python 3.6+ code like f-strings, unless you specifically setpython_versioninmypy.ini.We apply the same fix as above so that MyPy now also works "out of the box" with Python 3.6-only and 3.7-only codebases. If users set
python_versioninmypy.ini, MyPy will respect this - we only influence the default behavior.Result
Toolchain's Python 3.8 codebase now works "out of the box" with Pants, i.e. without extra settings.
[ci skip-rust]
[ci skip-build-wheels]