gh-89828: Do not relay the __class__ attribute in GenericAlias #93754
Conversation
list[int].__class__ returned type, and isinstance(list[int], type) returned True. It caused numerous problems in code that checks isinstance(x, type).
serhiy-storchaka
added
type-bug
serhiy-storchaka
requested review from
gvanrossum,
Fidget-Spinner,
JelleZijlstra,
ericvsmith and
rhettinger
as code owners
The code LGTM. I'd prefer to wait a bit to hear what the others have to say though.
|
Alternatively, we can stop relaying any dunder name, except |
|
I agree with this idea but we need to think about compatibility implications. We can definitely make the change only on main. |
|
3.11 has not been released yet. I planned to do this change in 3.11, but there were tasks with more priority before the first beta. There were many changes in GenericAlias related to PEP 646, and the work is not finished yet. I suspect that keeping this behavior not fixed in 3.11 can cause new 3.11 specific bugs. It will be easier if the code of 3.11 and main be synchronized. @pablogsal |
|
I'm fine backporting this as this is in the realm of what we can consider a bugfix if nobody has any concerns. But at least another two core Devs must agree with the backport (by approving it). Personally I think it makes sense to have it on 3.11 to stop people relying on the old behaviour and "stop the bleeding". Given they this is quite specific I think we are still on time. Please, also tag me in the backport PR if we decide to move forward with it. |
|
It's unfortunately we waited so long. Not doing this in 3.11 makes it more likely that there will be code relying on the old behavior (no matter how strange). Alas, the old behavior was released with 3.9 and 3.10 (though some bugs related to this weren't fixed in those releases), so I'm not sure it's that important to fix it in 3.11 -- people are going to have to fix their code regardless. (It would seem that most of the time code that works around this will keep working though -- most of the fixes in this PR are removing workarounds, but that's cleanup -- that code will work with or without the workarounds. So I'll wait for @pablogsal. |
|
Workarounds for known issues in the stdlib were backported to 3.10 and 3.9 (for these for which I had found reproducers). The third-party code can still be affected, of course. |
The change looks good to me.
I still think we should change this only in 3.12. The old behavior was bad, but it was already that way in 3.9 and 3.10, so there's little harm in waiting a bit longer. However, if others think it's worth changing this in 3.11, I'd be OK with backporting.
Misc/NEWS.d/next/Core and Builtins/2022-06-12-19-31-56.gh-issue-89828.bq02M7.rst
Outdated
Show resolved
Hide resolved
…e-89828.bq02M7.rst Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
|
Why is it good to fix this in 3.12, but is not good to fix this in 3.11? I am now going through the stdlib code and have found many other places affected by this bug. I am writing tests for them. |
|
I think we should backport to 3.11. Not many users use the betas (compared to the actual releases anyways). |
|
I agree with Serhiy. I think the sooner this is fixed, the better, as it will reduce the possibility that third-party code starts to rely on It's unfortunate this is happening after betas have been released, but I still think it's worth it going into 3.11. |
|
Backporting to 3.11 will also fix #93847, which is a bug that's new in 3.11. |
|
I was worried by a changed deep in the beta cycle but it's very likely okay, so go ahead. |
|
Okay what are we waiting for? |
|
I was going to add yet few tests and fixes, backport them down to 3.10, then merge this PR and backport it to 3.11. It would be less work than creating different PRs for 3.10 and 3.11-main. |
|
Thanks @serhiy-storchaka for the PR |
…ythonGH-93754) list[int].__class__ returned type, and isinstance(list[int], type) returned True. It caused numerous problems in code that checks isinstance(x, type). (cherry picked from commit f9433ff) Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
|
GH-93980 is a backport of this pull request to the 3.11 branch. |
|
Merged, because my computer crashed, and I lost all local changes. |
) list[int].__class__ returned type, and isinstance(list[int], type) returned True. It caused numerous problems in code that checks isinstance(x, type). (cherry picked from commit f9433ff) Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
list[int].__class__returnedtype, andisinstance(list[int], type)returned
True. It caused numerous problems in code that checksisinstance(x, type).#89828