Repository navigation
Require abstract classes in stubs to be explicitly marked - #5223
Conversation
|
I totally like the idea in principle, but what to do with the test failures (mypy uses many test fixtures)? Maybe limit this to only typeshed stubs (in contrast to all stubs)? There is a corresponding flag in |
|
I think the failures in mypy's stubs won't be too hard to fix. The |
|
I think this can be merged after you are done with typeshed updates and it is synced. |
This is needed to make TextIOWrapper and a few classes that inherit from it concrete in 3.3. I am actually not sure whether this method exists at runtime; there's nothing in https://docs.python.org/3/library/io.html#io.TextIOWrapper that suggests it was added in 3.4. In any case, it hardly matters since 3.3 usage should be very rare now. This appears to be the last typeshed change needed to unblock python/mypy#5223. @ilevkivskyi would you mind reviewing?
|
Going to close and re-open to see if that forces Appveyor to re-run (Travis already passed). |
|
All tests pass now, but I would wait until tomorrow to see if there any objections from @JukkaL |
|
@JukkaL do you have any feedback here? (Also, thanks for fixing the merge Ivan.) |
|
FWIW this PR passes |
|
I assume the silence means no. |
2 participants
Fixes #3692.
In typeshed, we have had many bugs because of classes that were not meant to be abstract, but were inferred by mypy to be abstract because they didn't implement some methods from an abstract base class. I call such classes "accidentally abstract"; python/typeshed#1476 has a list of cases where this bug appeared.
It is hard to catch these bugs because they only show up in user code when somebody tries to instantiate the class, and when reviewing changes to typeshed you have to manually go over all methods to find instances of this bug. Therefore, I'd like to have mypy point out accidentally abstract classes automatically.
This PR throws an error if a class:
@abstractmethod(i.e., all abstract methods are inherited);ABCMetaas an explicit metaclass, orProtocolas an explicit base class;This generates about five false positives in typeshed's Python 3 code, which we can fix by manually adding
ABCMetaas a metaclass. On the other hand, there have been at least 30 accidentally abstract classes in typeshed that would have been caught by this patch, although I've fixed most of them by now.This PR is going to fail tests for now because not all typeshed classes are fixed yet. If we're in agreement that this PR is acceptable, I will work on getting typeshed ready for it by adding the right
ABCMetametaclasses.