Skip to content
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

PEP 585 breaks inspect.isclass #88459

Closed
wyfo mannequin opened this issue Jun 2, 2021 · 9 comments
Closed

PEP 585 breaks inspect.isclass #88459

wyfo mannequin opened this issue Jun 2, 2021 · 9 comments
Labels
3.9 only security fixes topic-typing

Comments

@wyfo
Copy link
Mannequin

wyfo mannequin commented Jun 2, 2021

BPO 44293
Nosy @gvanrossum, @ilevkivskyi, @JelleZijlstra, @wyfo, @Fidget-Spinner

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2021-06-02.22:26:01.666>
labels = ['3.9']
title = 'PEP 585 breaks inspect.isclass'
updated_at = <Date 2021-06-05.22:35:28.114>
user = 'https://github.com/wyfo'

bugs.python.org fields:

activity = <Date 2021-06-05.22:35:28.114>
actor = 'levkivskyi'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = []
creation = <Date 2021-06-02.22:26:01.666>
creator = 'joperez'
dependencies = []
files = []
hgrepos = []
issue_num = 44293
keywords = []
message_count = 8.0
messages = ['394950', '394963', '394966', '394988', '395001', '395006', '395008', '395185']
nosy_count = 5.0
nosy_names = ['gvanrossum', 'levkivskyi', 'JelleZijlstra', 'joperez', 'kj']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = None
url = 'https://bugs.python.org/issue44293'
versions = ['Python 3.9']

@wyfo
Copy link
Mannequin Author

wyfo mannequin commented Jun 2, 2021

PEP-585 has the side-effect of making list[int] an instance of type. This is not the case for other generic aliases.

It also implies that inspect.isclass(list[int]) is True, while list[int] is not a class; as a proof of this statement issubclass(list[int], collections.abc.Collection) raises TypeError: issubclass() arg 1 must be a class.

By the way, there is the awkward thing of having isinstance(list[int], type) is True while issubclass(type(list[int]), type) is False.

@wyfo wyfo mannequin added the 3.9 only security fixes label Jun 2, 2021
@JelleZijlstra
Copy link
Member

The reason for this is that types.GenericAlias.getattribute delegates to the alias's origin (in the ga_getattro function). As a result, list[int].__class__ calls list.__class__ and returns type. And the implementation of isinstance(obj, type) ultimately calls issubclass(obj.__class__, type). (That's in object_isinstance() in abstract.c. It's news to me; I didn't know you could customize isinstance() behavior on the object side.)

To fix this, we could make ga_getattro not delegate for __class__, so that list[int].__class__ would return GenericAlias instead of type. The current implementation of GenericAlias has been around for a few releases by now, though, so that change might break some use cases.

This is not the case for other generic aliases.

This is not true; it is the same for e.g. set[int]. Unless you meant something else here.

@gvanrossum
Copy link
Member

Since these are new forms (list[int] previously was an error), does it actually matter? Especially since these are primarily used in annotations.

@joseph Perez, is there a specific library or pattern that is broken by this?

FWIW I did think rather carefully about which attributes to delegate or not, and delegating __class__ was intentional.

@wyfo
Copy link
Mannequin Author

wyfo mannequin commented Jun 3, 2021

@jelle Zijlstra Thank you for the explanation.

The current implementation of GenericAlias has been around for a few releases by now, though, so that change might break some use cases.

I agree that a "fix" could have unexpected side-effect, my issue comes quite late indeed. By the way, Python typing is so much unstable (every version breaks the previous one), it's very complicated to write code that support multiple versions, so whatever the typing internal implementation, we must adapt.

This is not true; it is the same for e.g. set[int]. Unless you meant something else here.

I have chosen list[int] as an example of types.GenericAlias introduced by PEP-585 (i could have chosen set[int] or collections.abc.Collection[int]). But other generic aliases, e.g. typing.List[int] or MyClass[int] (where MyClass inherits Generic[T]), are not instances of type.

@joseph Perez, is there a specific library or pattern that is broken by this?

Because issubclass requires a "class" as arg 1, I use the pattern if isinstance(tp, type) and issubclass(tp, SomeClass) (isinstance check being equivalent to inspect.isclass). With PEP-585, it breaks for list[int] and other builtin generic aliases.

FWIW I did think rather carefully about which attributes to delegate or not, and delegating __class__ was intentional.

I don't have the context of the decision, so I can quite understand that delegating __class__ was the right thing to do, especially when __mro__ and other type attributes are also delegated.
I mainly wanted to highlight this side effect, especially on the pattern mentioned above. (My issue title is a little bit excessive in this regard)

But as I've written, I've already so many wrappers to maintain compatibility between Python versions of typing that I can write a new one to handle this particularity of PEP-585. So this issue is not critical to me.

@Fidget-Spinner
Copy link
Member

@jelle thanks for nosy-ing me too and the thorough investigation.

@joseph

Thanks for taking the time to raise this inconvenience on the bug tracker.

By the way, Python typing is so much unstable (every version breaks the previous one), it's very complicated to write code that support multiple versions, so whatever the typing internal implementation, we must adapt.

Compared to some of the more mature modules in Python, I have to agree that typing.py is mildly unstable. However, you're not supposed to be using/importing from the internal constructs - those have no guarantee of stability. If you feel some common use cases aren't met by the current introspection helpers, please please please create a new issue for that and we'll consider it. How we use typing may differ from how you use it and so there's a lot we don't see. User bug reports and feedback have helped to surface such issues and improved typing for everyone :).

I have chosen list[int] as an example of types.GenericAlias introduced by PEP-585 (i could have chosen set[int] or collections.abc.Collection[int]). But other generic aliases, e.g. typing.List[int] or MyClass[int] (where MyClass inherits Generic[T]), are not instances of type.

This is an implementation detail. Most typing PEPs don't usually specify the runtime behavior in detail because most of them focus on static analysis. The implementation is usually up to the contributor's judgement. FWIW, to accommodate the new 3.9 GenericAlias, typing.py just added additional checks for isinstance(tp, types.GenericAlias) instead of checking only for isinstance(tp, type) and friends.

@gvanrossum
Copy link
Member

Instead of introspecting types, use this library:
https://github.com/ilevkivskyi/typing_inspect

@ilevkivskyi
Copy link
Member

Btw this reminds me I should make a PyPI release of typing_inspect (last release was May 2020), hopefully will make a release on this weekend.

@ilevkivskyi
Copy link
Member

Uploaded typing_inspect 0.7.0 to PyPI (it should work with Python 3.9 hopefully)

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
@JacobHayes
Copy link
Contributor

I think this can be closed after #93754 (though I don't see it in the 3.11 changelog):

Python 3.9.14 (main, Nov 28 2022, 13:10:52)
[Clang 13.1.6 (clang-1316.0.21.2.5)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from types import GenericAlias
>>> isinstance(list[int], type)
True
>>> isinstance(list[int], GenericAlias)
True
Python 3.11.0 (main, Nov 28 2022, 13:26:47)
[Clang 13.1.6 (clang-1316.0.21.2.5)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from types import GenericAlias
>>> isinstance(list[int], type)
False
>>> isinstance(list[int], GenericAlias)
True

Anyone relying on isinstance(list[int], type) to be True can instead use isinstance(list[int], (type, GenericAlias)).

masenf added a commit to reflex-dev/reflex that referenced this issue Mar 26, 2024
In python 3.9 and 3.10, the `isinstance(list[...], type)` returns True, but
it's not a valid class for use in issubclass
masenf added a commit to reflex-dev/reflex that referenced this issue Mar 27, 2024
…s. (#2893)

* When a Var points to a model, prefer access to model fields.

When a Var points to a model, and fields of the model share the same name as
Var operations, access to the model fields is now preferred to avoid having Var
operation names shadow model fields. Since most Var operations do not actually
work against Models, this does not really block any functionality.

* Special case for ComputedVar needing to internally access fget

Since fget is a "slot" on property, normal __getattribute__ access cannot find it.

* Workaround python/cpython#88459

In python 3.9 and 3.10, the `isinstance(list[...], type)` returns True, but
it's not a valid class for use in issubclass
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.9 only security fixes topic-typing
Projects
None yet
Development

No branches or pull requests

6 participants