classification
Title: Shouldn't `typing.get_type_hints()` default `globalns` to `{}` instead of skipping base classes?
Type: behavior Stage: resolved
Components: Versions: Python 3.11, Python 3.10
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Jelle Zijlstra, WCA, gvanrossum, kj, levkivskyi, miss-islington
Priority: normal Keywords: patch

Created on 2021-06-21 04:11 by WCA, last changed 2021-06-27 02:32 by gvanrossum. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 26862 merged WCA, 2021-06-22 23:11
PR 26920 merged miss-islington, 2021-06-26 23:31
Messages (7)
msg396205 - (view) Author: Will Chen (WCA) * Date: 2021-06-21 04:11
An issue was recently closed that caused synthetic classes and base classes with invalid `__module__` attributes to raise `KeyError()` in `typing.get_type_hints()`:

https://bugs.python.org/issue41515

However, the implemented solution appears to be to skip those classes completely with a `continue` statement, instead of getting the annotations that may still be present by using an empty globals dictonary:

https://github.com/python/cpython/pull/25352/files#diff-ddb987fca5f5df0c9a2f5521ed687919d70bb3d64eaeb8021f98833a2a716887

In order to work around this issue in my local install of Blender, I had to change `.get_type_hints()` to use an empty dictionary for `globalns` when encountering invalid modules, rather than skipping them:

https://developer.blender.org/T88986#1179812

From reading the commit where the broken behaviour was first introduced— Which was described/designed as "backwards compatible"— It looks like the original behaviour was also to use an empty dictionary, and never skip:

https://github.com/python/cpython/commit/f350a268a7071ce7d7a5bb86a9b1229782d4963b#diff-ddb987fca5f5df0c9a2f5521ed687919d70bb3d64eaeb8021f98833a2a716887R1501

Using an empty dictionary also seemed to be mentioned in the bug report linked above.

IMO using an empty dictionary and still returning annotations from classes with invalid modules seems like it'd be more sensible, predictable, and backwards-compatible, while skipping base classes is likely to just replace the obvious `KeyError()` with less reproducible and nastier errors caused by returning incomplete type hints.
msg396222 - (view) Author: Ken Jin (kj) * (Python committer) Date: 2021-06-21 10:26
Will, I think what you say makes sense.

@Guido / Ivan / Jelle

What do you think?
msg396279 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2021-06-21 18:16
I agree with the OP, we can do better here. Ken, if you feel like it, a fix
should not be too complicated, right?
msg396315 - (view) Author: Ken Jin (kj) * (Python committer) Date: 2021-06-22 09:43
> Ken, if you feel like it, a fix should not be too complicated, right?

Yup. Will's proposal is to change the try-except to:

base_globals = getattr(sys.modules.get(base.__module__, None), '__dict__', {})

@Will, would you like to submit a PR for this? You'd need to sign a CLA. However, if you'd prefer I do it instead I'm happy to make the change.

3.10.0 beta 4 is releasing on 2021-07-10, so preferably we should get the change in by then. It's the last beta before the release candidates.
msg396376 - (view) Author: Will Chen (WCA) * Date: 2021-06-22 23:33
I opened a PR with a fix and a couple comments on its method:

https://github.com/python/cpython/pull/26862

I also *think* I signed the CLA correctly. Not sure. I used my login name here, rather than my Github login, but my account is linked.

Didn't add any blurb/changelog notes. Would that be needed?

Thanks for looking at this issue quickly! Let me know if there's anything else I can or should do.
msg396559 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2021-06-26 23:31
New changeset 7569c0fe91dfcf562dee8c29798ecda74d738aa8 by will-ca in branch 'main':
bpo-44468: Never skip base classes in `typing.get_type_hints()`, even with invalid `.__module__`. (GH-26862)
https://github.com/python/cpython/commit/7569c0fe91dfcf562dee8c29798ecda74d738aa8
msg396561 - (view) Author: miss-islington (miss-islington) Date: 2021-06-26 23:52
New changeset 3df23b5199a4bb237a595cadca6c49d34ab2a56c by Miss Islington (bot) in branch '3.10':
[3.10] bpo-44468: Never skip base classes in `typing.get_type_hints()`, even with invalid `.__module__`. (GH-26862) (GH-26920)
https://github.com/python/cpython/commit/3df23b5199a4bb237a595cadca6c49d34ab2a56c
History
Date User Action Args
2021-06-27 02:32:00gvanrossumsetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2021-06-26 23:52:34miss-islingtonsetmessages: + msg396561
2021-06-26 23:31:42gvanrossumsetmessages: + msg396559
2021-06-26 23:31:41miss-islingtonsetnosy: + miss-islington
pull_requests: + pull_request25493
2021-06-22 23:33:18WCAsetmessages: + msg396376
2021-06-22 23:11:30WCAsetkeywords: + patch
stage: patch review
pull_requests: + pull_request25437
2021-06-22 09:43:18kjsetmessages: + msg396315
2021-06-21 18:16:03gvanrossumsetmessages: + msg396279
2021-06-21 10:26:55kjsetnosy: + kj, gvanrossum, Jelle Zijlstra, levkivskyi
messages: + msg396222
2021-06-21 04:11:39WCAcreate