-
Notifications
You must be signed in to change notification settings - Fork 237
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix generics inheritance and drop __subclasscheck__ #207
Conversation
c12d92f
to
bbeadb1
Compare
I like this PR, it looks like it could fix many issues. But I have some questions:
|
@ilevkivskyi The short answer to 1 and 3 is that, when it comes to certain details, the PR represents a WIP due to the exact requirements not being known.
In the context of the full PR, the equality becomes very questionable. And even if we do want it to hold, the duplication won't be necessary. (The conversion of extras into bases will no longer skip builtins here.) EDIT: Looking at this again, I recalled the reason for not eliminating the duplicates yet. It's |
Superseded by #283. Many of the ideas here went into that PR, and it closes most of the same issues. Thanks! |
Here I am adding tests as discussed with @bintoro (related to #207). These tests actually revealed three small bugs: - Old style classes in Python don't have `__mro__`. - We should use `__extra__` instead of extra in Python2 (since no kwargs in classes). - We should allow overriding `__subclasshook__` by subclasses.
I don't expect this to be reviewed anytime soon, but the effort is now fairly complete, so might as well put it out there for comments.
Resolves #136
Resolves #202
Resolves #203
At the heart of #136 is a conflation between type compatibility and subtype relationships. Even though
Sequence[Manager]
is compatible withSequence[Employee]
, it's not a subclass of it. Rather, both derive fromSequence[+T_co]
.To address this, the PR introduces a new helper
is_compatible()
, which implements the "is consistent with or a subtype of" relation. In short, it does whatissubclass()
has been doing until now.The practical end result is that
issubclass()
is no longer applicable to the special typesAny
,Union
, etc.is_compatible()
always works — if neither argument is a special type,is_compatible(T1, T2)
reduces toissubclass(T1, T2)
issubclass()
acquires its normal semantics w.r.t. generics:whereas:
Commit summary
There are three commits:
__extra__
linkage from leaking down to generics' subclasses. This fixes the bug whereissubclass(dict, CustomMapping) == True
. [Diff]__extra__
ABCs as actual base classes of theirtyping
twins. It also refactorsGenericMeta.__subclasscheck__
into a compatibility check and a__subclasshook__
dedicated to handling the__extra__
connection. [Diff] This achieves a few things:collections
ABCs.collections.abc
become available.issubclass(list, Iterable[X])
no longer returns true for arbitraryX
(Kill __subclasscheck__ #136).TypeError
in response toissubclass()
. Every type metaclass gets a method named__is_compatible__
to host the prior__subclasscheck__
logic. [Diff]Notes
Full parity is preserved in using unparameterized types in place of the corresponding
collections.abc
classes inisinstance()
andissubclass()
calls:The intent has been not to mess with the types' behavior. That said, the introduction of
is_compatible()
yields a couple of automatic improvements regardingAny
:Any
; previouslyissubclass(Any, C)
would return false for some arbitrary classC
but true for atyping
type.is_compatible(Iterable[Any], Iterable[int])
correctly returns true.The merging of Specific asserts #205 caused a huge conflict in the tests. I will look into it eventually if there's interest in merging this.
The proposal is compatible with the plan to convert some of the collection types to protocols, if that's still on the table.