https://pantsbuild.org/ logo
Join Slack
Powered by
When running `check` on python code with [mypy].in...
# general
h
When running 
check
on python code with [mypy].interpreter_constraints is set, we don't actually use those constraints on targets that have their own ICs
So there is no way to say "run mypy using Python 3.9" on code that may be compatible with earlier versions
I'm wondering if setting 
[mypy].interpreter_constraints
explicitly should just bypass the partitioning and all that, and just do the simple and obvious thing that is implied by that
?
Or rather, presumably there is a reason it doesn't do that, but it's not clear what it is, and right now it looks like we simply ignore a config setting
e
As a datapoint I find useful over in Pex - I run mypy with an effectively arbitrary interpreter (that's compatible with mypy of course), but I run it over most python versions my code supports (I pick 4 versions, not all) using 
mypy --python-version X.Y
. In the Pex case this would be like using the Pants mypy ICs to create the right MyPy PEX tool, and then running the tool over ~each Python major.minor implied by the code being checked ICs. And in the Pex case this is important since Pex works with 2.7 which has different type checking concerns than 3.x - namely around unicode vs str. I'm not sure if that generalizes to differences in 3.x version or not and I'm pretty darn sure its not performant to just auto-cover the IC range like that.
But even if running over the range isn't performant, the Pex example points to a fail-safe algorithm: Use the MyPy ICs to build the MyPy PEX tool, pick the minimum Python major/minor version from the code-under-check ICs and run MyPy once with that 
--python-version
.
h
@happy-kitchen-89482 Pants lets you explicitly set 
python_version
to force a version
e
So Benjy is observing a big then. If the code under check is Python 2.7 code, that code's IC will be used for the MyPy PEX tool, which will fail since MyPy should be run using some Python 3 but told 
--python-version 2.7
h
I vaguely remember them changing something about running w/ Py2 recently - but when this code was written, not so thanks to 
typed_ast
. MyPy could understand prior Python versions than the interpreter it was run with
e
It does as Pex use shows. I use a Python 3.10 MyPy venv to run against 
--python-version {2.7,3{5,10,11}}
This is at least since MyPy 0.800 Feb 2021: https://github.com/pantsbuild/pex/pull/1233
h
Well, 
python_version
is something different - it's the version targeted by mypy, not the version we run mypy on
But that's not what we partition on
we partition on ICs (and resolves) and then try to set 
--python-version
dynamically in each partition, unless it's already set in mypy.ini or args
But I've set 
[mypy].interpreter_constraints
explicitly to Python 3.9 yet I still get partitioning, and one of the partitions runs mypy on 3.6, because that code has permissive constraints
Even though it would be fine (and in fact required in this case) to run on 3.9 everywhere in a single partition
I know I keep coming back to this, but this is a manifestation of the complexity with treating ICs as two different things: 1) descriptive metadata about the compatibility of some .py files, and 2) imperative config for running a tool on that code
BTW since I set 
[mypy].interpreter_constraints
explicitly, the tool lockfile is generated for those constraints, so running it on other constraints might not even work
So it seems like if you set 
[mypy].interpreter_constraints
explicitly we should always run with that, but still partitioning if we're setting 
python_version
dynamically, and if you set 
python_version
explicitly in 
mypy.ini
then we shouldn't partition at all?
And we probably shouldn't warn about the explicit python_version in 
mypy.ini
unless there's a conflict. It's weird to warn about normal use of a tool config file.
Although actually even using 
[mypy].interpreter_constraints
isn't right - those are constraints for generating a lockfile, and we may want a lockfile compatible with multiple versions of mypy (in this case because we need to run mypyc at package time on several different interpreters)
But we would still like a way to say "when running mypy as a tool, do it on this interpreter version"
e
Well, 
python_version
is something different - it's the version targeted by mypy, not the version we run mypy on
Absolutely agreed. We need just 2 things: 1.) A valid Python to run MyPy with - this need have 0 to do with what Python your code under check needs to run. 2.) --python-version to say which interpreter to run the check using The partitioning and other complications are just that. I have 0 clues what that is / what we do there, but the above 2 facts need to be the basis.
➕ 1
h
Yeah, good summary John. 
[mypy].interpreter_constraints
is meant to be #1. Partitioning + the allowance for 
python_version
is meant to be #2
h
So I think right now those two get more mingled up than we would like
In that we often ignore 
[mypy].interpreter_constraints
and use the code's constraints instead
For #1
h
We only do that for #1 because of the typed-ast limitation. If you run MyPy with Py37, it cannot understand Py38+ code. Same story w/ Black Note we only ignore the option if it was not explicitly st
h
But if the user has explicitly said "run on this version" then we should probably do that?
h
If they explicitly set it, then no matter what we use the option
h
As far as I can tell that is not the case
h
h
Hmm, so I'm not sure what I'm seeing
h
To make clear: the intent w/ #1 is that users don't need to worry about it, as it's ~implementation detail. Pants Just Does The Right Thing Modeling for #2 could probably be improved tho. Feel free to re-envision the partitioning
h
The only thing with #2 is not to warn about not being able to set that explicitly, unless there's a conflict perhaps
If you have an existing mypy.ini that sets python_version, it's odd to get a warning
The partitioning I'm getting is correct, just possibly not necessary
Since in the end both partitions are running with the same interpreter, the same python_version and the same resolve
➕ 1
But I don't think that's a big deal
2 Views
Open in Slack
PreviousNext
Powered by