Repository navigation
Add a mypy plugin to support @total_ordering. - #9525
Conversation
Without this, when using a comparison method synthesized by the
`@total_ordering` class decorator, MyPy will incorrectly observe
something like:
```
src/python/pants/option/option_value_container.py:108:12: error: Unsupported
left operand type for >= ("Rank") [operator]
if new_rank >= existing_rank:
^
Found 1 error in 1 file (checked 40 source files)
```
Here, just `__lt__` is explicitly defined in the "Rank" class and
`@total_ordering` generates `__ge__` which MyPy is ignorant of.
Also, in order to get the in-repo plugin working, fix the mypy task to
upgrade its interpreter when `--include-requirements` is turned on and
remove type checking tags from mypy example targets which are only ever
used in integration tests where tags are not used as filters anyhow.
[ci skip-jvm-tests]
|
A follow-up that converts RankedValue rank integer constants to an Enum will use this to pass type checks without compromise... besides this compromise. |
Eric-Arellano
left a comment
Eric-Arellano
left a comment
There was a problem hiding this comment.
Huh, I've never seen a MyPy plugin before. Neat!
Consider trying to upstream this - I think it'd be helpful to others and MyPy seems receptive to contributions like this.
| cmd = [ | ||
| "--backend-packages=pants.contrib.mypy", | ||
| f'--mypy-config={cls.example_dir / "mypy.ini"}', | ||
| "--mypy-version=mypy==0.720", |
There was a problem hiding this comment.
What's the motivation for this change?
There was a problem hiding this comment.
This test tests the Django MyPy plugin and that plugin doesn't work with our new, newer, 3rdparty dep so I pinned to the low point of the range it does support. IIRC it was >=0.720,<0.730.
|
How will this interact with the upstreaming of a potential v2 implementation of mypy? |
|
@Eric-Arellano it looks like this is already underway: |
Thats alot of speculation that'd I'd normally defer on until something real showed up, but I assume you're being coy about Toolchain's internal v2 impl which actually exists and is slated for upstreaming. Good question. There is a small fix in this PR for MypyTask handling of the interpreter to run mypy with when After that fix, the only other requirement - which is a requirement in v1 here too - is that the plugin registration function is available on the sys.path for MyPy to find. Since v1 adds all python source roots to the PYTHONPATH (not sure why it does this!), all v1 invocations see the plugin registration function since its in an in-repo loose source form. In v2 we'll need to add a dependency on our MyPy plugin. This will require one of:
Option 3 is what we want at the end of the day (The |
Without this, when using a comparison method synthesized by the
@total_orderingclass decorator, MyPy will incorrectly observesomething like:
Here, just
__lt__is explicitly defined in the "Rank" class and@total_orderinggenerates__ge__which MyPy is ignorant of.Also, in order to get the in-repo plugin working, fix the mypy task to
upgrade its interpreter when
--include-requirementsis turned on andremove type checking tags from mypy example targets which are only ever
used in integration tests where tags are not used as filters anyhow.
[ci skip-jvm-tests]