◐ Shell
reader mode source ↗
Skip to content

Kill __subclasscheck__#283

Merged
gvanrossum merged 28 commits into
python:masterfrom
ilevkivskyi:master
Sep 27, 2016
Merged

Kill __subclasscheck__#283
gvanrossum merged 28 commits into
python:masterfrom
ilevkivskyi:master

Conversation

@ilevkivskyi

@ilevkivskyi ilevkivskyi commented Sep 18, 2016

Copy link
Copy Markdown
Member

This PR:

The idea is to make most things not classes. Now _ForwardRef(), TypeVar(), Union[], Tuple[], Callable[] are not classes (i.e. new class objects are almost never created by typing).

Using isinstance() or issubclass() rises TypeError for almost everything. There are exceptions:

  • Unsubscripted generics are still OK, e.g. issubclass({}, typing.Mapping). This is done to (a) not break existing code by addition of type information; (b) to allow using typing classes as a replacement for collections.abc in class and instance checks. Finally, there is an agreement that a generic without parameters assumes Any, and Any means fallback to dynamic typing.
  • isinstance(lambda x: x, typing.Callable) is also OK. Although Callable is not a generic class, when unsubscribed, it could be also used as a replacement for collections.abc.Callable.
  • The first rule for generics makes isinstance([], typing.List) possible, for consistency I also allowed isinstance((), typing.Tuple).

Finally, generics should be classes, to allow subclassing, but now the outcome of __getitem__ on classes is cached. I use an extended version of functools.lru_cache that allows fallback to non-cached version for unhashable arguments.

This is still WIP, since I did this only for Python 3.

@gvanrossum @JukkaL Please take a look.

@ilevkivskyi

Copy link
Copy Markdown
Member Author

@gvanrossum @JukkaL
It looks like this PR also fixes http://bugs.python.org/issue26075, http://bugs.python.org/issue25830, and http://bugs.python.org/issue26477

I added this to the description.

@JukkaL

JukkaL commented Sep 20, 2016

Copy link
Copy Markdown
Contributor

I did a quick pass on the diff and tried it with mypy, and I like it. The changes seem to speed up a no-op mypy invocation (mypy) by roughly 20%, which is nice (though I didn't do very careful benchmarking). There still seems to be maybe a 5-10% slowdown in startup time due to type annotations, but this seems acceptable already.

The generic type object cache hit rate when running mypy -x was about 78% -- increasing the cache size to 50 improved that to 80%. A further increase only had a marginal effect. The improvement with a larger cache might be higher to larger codebases, but it's hard to tell.

@gvanrossum gvanrossum left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hide comment

This is great! Here are some nits. My main concern is over __slots__ but even that's minor. I haven't reviewed everything (in fact I only reviewed src/typing.py) but I think we should just go ahead and release this with 3.6b2 and hope for the best. Maybe we'll get some real-world feedback from that and we can tweak things before 3.5.3 is finalized (sorry, there's no date in PEP 478, but we'll get ample warning, and I expect it to be around the same time as 3.6.0 goes out, mid December according to PEP 494).

@ilevkivskyi ilevkivskyi changed the title [WIP] Kill __subclasscheck__ Sep 26, 2016
@ilevkivskyi

Copy link
Copy Markdown
Member Author

@gvanrossum
Thank you for review! I agree with all comments and implemented them (also in /python2). Except for one: I added _ to _TypingBase and _FinalTypingBase as you requested, but not to TypingMeta, there are few reasons for this:

  • It was around for a long time (probably since beginning).
  • Adding underscore there would probably also require adding a lot of them to TupleMeta etc. in /python2.
  • I think we could keep TypingMeta as a part of "semi-public" API (just in case someone wants to make extensions to typing module or mypy).

@ilevkivskyi

Copy link
Copy Markdown
Member Author

@JukkaL
I agree that the cache size should be increased. The idea is that projects that will use typing and mypy are probably very large (this is also Guido's opinion). Now the cache size is 128.

@gvanrossum gvanrossum left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hide comment

I am going to merge this as-is. While there are a few things left to do, it's easier to do them in a follow-up PR -- this one is just too big to review already.

@gvanrossum

Copy link
Copy Markdown
Member

Feel free to iterate more on this! Especially the Union[Any, int] issue. But I figured we might as well take a checkpoint, so this is now merged (also into CPython 3.5, 3.6 and 3.7==default). I closed the three CPython bugs you mentioned.

@ilevkivskyi

Copy link
Copy Markdown
Member Author

@gvanrossum
Thank you for merging this!
I will submit a new PR for Union[int, Any] in typing.py and probably also PR for PEPs 484 and 483.

This is a good catch, I didn't notice this while editing PEP 483. The point is that type is characterized by two sets: set of values and set of functions (methods), as mentioned in PEP 483. With such definition neither int is not a subtype of Any (since int has less methods) nor vice versa (since Any has more values). The fact that Any is at the same time at the top of type hierarchy (it has all values) and at its bottom (it has all methods) requires us to introduce two different concepts: subtype and compatibility.

Actually, now I think that Any should be considered a subtype of object. Indeed, both have all values, but Any has more methods, therefore it is a subtype of object. This, in turn, means that Union[object, Any] should be simplified to just object. For example, if x has type Union[object, Any] then the only way to call x.lower() is when x is not object, but this is not possible:

x: Union[object, Any]
x.lower()  # this should be rejected
if not isinstance(x, object):
    x.lower()  # this should be allowed, but it is not reachable

What do you think?

(If it will help, the relation between object and Any is similar to relation between, e.g., Mapping and MutableMapping, they both have the same set of values, but MutableMapping has more methods, therefore it is a subtype of Mapping.)

@gvanrossum

gvanrossum commented Sep 28, 2016 via email

Copy link
Copy Markdown
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants