bpo-44468: Never skip base classes in typing.get_type_hints(), even with invalid .__module__.

[will-ca]

https://bugs.python.org/issue44468

• Added test to make sure base classes with invalid modules still get their annotations picked up in get_type_hints().

• Moved tests to right place (was originally under class ClassVarTests, are now under class GetTypeHintTests).

• Used empty dict for globalsn instead of skipping class if its .__module__ is invalid.

---

I'm using default values for sys.modules.get() and then getattr(), to default to an empty dict.

I would probably actually personally consider it cleaner and clearer (with fewer function calls and objects) to either keep the try/except KeyError and just replace the continue with an assignment to an empty dict, or do an if/else check for containment in sys.modules.

But this approach is what I linked to on BPO-44468, and it does have the (somewhat dubious) additional benefit that it will still work and default to an empty dict even if someone adds a matching fake module to sys.modules that doesn't have a .__dict__ attribute.

I'm sure it's a minute detail that no one else necessarily cares about. But this is pretty much my second time ever contributing code to an outside project, and also the biggest chance I've had to accidentally break software that other people rely on (however small of a part of that software it may be)— So I just want to make sure I mention that to be considered.

https://bugs.python.org/issue44468

[the-knights-who-say-ni]

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA).

CLA Missing

Our records indicate the following people have not signed the CLA:

@will-ca

For legal reasons we need all the people listed to sign the CLA before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

If you have recently signed the CLA, please wait at least one business day
before our records are updated.

You can check yourself to see if the CLA has been received.

Thanks again for the contribution, we look forward to reviewing it!

[Fidget-Spinner]

Thanks for the quick turnaround! I'll review this soon. But a preliminary check LGTM. The CLA takes a few days to go through because its done manually. And I think this needs a blurb news entry please. The behavior is slightly different from previous versions.

Whether to try-except and assign empty dict or use the current method is up to you. I think it's fine as long as the specific use case you mentioned (bad module with some valid annotations) works.

[Fidget-Spinner]

Thanks for the excellent PR. Please add a news entry using blurb-it or blurb. It's step 8 here: https://devguide.python.org/#quick-reference

I recommend something along the lines of

:func:`typing.get_type_hints` now finds annotations in classes with unexpected ``__module__``. Previously, it would return an empty dictionary.

of anything you prefer.

BTW, if you would like to, you can add yourself to Misc/ACKS as well https://github.com/python/cpython/blob/main/Misc/ACKS . If you don't want to that's fine too :).

[will-ca]

Thanks for the review!

I thought the behaviour is slightly different too, but actually I'm not sure it is, compared to previous releases.

All I've done is re-implement the behaviour described in GH-25352/BPO-41515, in a slightly more robust way. So on that level, I think the blurb from there also describes everything this PR does.

The behaviour does change compared to that previous PR. But that last PR was only on April 12th, and neither the changelogs nor the Git branches for 3.9.5 and 3.8.10, the only releases since then (I think), seem to include it.

So this PR behaves slightly differently from a previous commit. But it only differs from any previous numbered versions/releases in ways that are already described by the blurb from that previous commit. It's not clear to me whether it'd be preferred to add a blurb which references changes that never actually made it into any releases, or to leave this commit technically undocumented.

[gvanrossum]

If it differs from a previous beta release of 3.10, it should have a news blurb. (The news blurbs are not the same as "What's new in 3.10".)

[will-ca]

Blurb added, tests passed, CLA signed and confirmed!

[miss-islington]

Thanks @will-ca for the PR, and @gvanrossum for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10.
🐍🍒⛏🤖

[bedevere-bot]

GH-26920 is a backport of this pull request to the 3.10 branch.

[Fidget-Spinner]

Thanks for the contribution @will-ca . Great work!