Fix for mypy 1.4 changes#83
Conversation
e0a930b to
d7f402e
Compare
80cc7b2 to
1f65b74
Compare
List of relevant changes: - A whole bunch of types that are final at runtime, are now final in for type checking purposes as well - Add missing attrs decorators to some types. Internal attrs members have been added to the allowlist - StrictFIFOLock is no longer a subclass of Lock, as Lock is suposed to be final. - Converted attributes to read-only properties whenever this was necessary to match trio Minor changes: - Ignore internal APIs and tests (previously seemed to be the case by default) - Typevars and aliases have been prefixed with underscores, so that they are not part of the public interface of the modules (as they are not available at runtime)
|
Could you check the mypy version we are passed to choose whether to pass I don't like having to drop mypy < 1.4 |
6c273a7 to
01a4c66
Compare
Running the tests is not supported on all previous versions of mypy back to 0.920, but otherwise the plugin should work fine
9b050c7 to
2adfc8e
Compare
|
I have added a check for the mypy version (and some other required changes due to changes in trio) |
A5rocks
left a comment
There was a problem hiding this comment.
Assuming the type changes are correct, this looks fine! I'd like someone who actually knows trio-typing to look at this before merging though :P
| "trio >= 0.16.0", | ||
| "typing_extensions >= 3.7.4", | ||
| "mypy_extensions >= 0.4.2", | ||
| "async_generator", |
There was a problem hiding this comment.
Why do you need this backport package?
There was a problem hiding this comment.
Because otherwise you have this error: Error importing plugin "trio_typing.plugin": No module named 'async_generator'
We should be able to bump `mypy` after the PR python-trio/trio-typing#83 is merged
We should be able to bump `mypy` after the PR python-trio/trio-typing#83 is merged
jakkdl
left a comment
There was a problem hiding this comment.
I'm no expert on trio-typing (is anybody?), but seems fine to me.
oremanj
left a comment
There was a problem hiding this comment.
Sorry for the delay in looking at this. I'm confused about one of the changes but that doesn't need to block merging.
| @final | ||
| @attr.s(eq=False, hash=False, slots=True) | ||
| class RunVar(Generic[_T], metaclass=ABCMeta): | ||
| _NO_DEFAULT = object() |
There was a problem hiding this comment.
I don't think stubtest should be insisting that we expose underscore-prefixed attributes...
There was a problem hiding this comment.
The idea is that these attributes don't exist in the real thing, but if we say they do in the stubs, typecheckers think that they do exist, which confuses everyone. It is best practice to underscore thinks that don't exist in the real version.
There was a problem hiding this comment.
It's not really stubtest's fault here. It needs these attributes to construct the correct __init__ with attrs. The error only complains about __init__ and __match_args__, not missing attributes. I can remove the _NO_DEFAULT and just inline the object() however.
|
Thanks for all the reviews! |
This PR contains all the necessary changes to the plugin, tests and stubs for mypy 1.4
Api changes
Stubtest changes
As stubtest is part of mypy, and therefore also got updated, the stubs required signficant changes:
List of relevant changes:
for type checking purposes as well
have been added to the allowlist
be final.
necessary to match trio
Minor changes:
default)
are not part of the public interface of the modules (as they are not
available at runtime)
Fixes #84
Fixes #85