get_type_hints(): find the right globalns for classes and modules #470
Conversation
ambv
force-pushed
the
globalnsForClasses
branch
from
3e80ba7 to
9378f79
Compare
src/typing.py
Outdated
| @@ -1494,7 +1495,10 @@ def get_type_hints(obj, globalns=None, localns=None): | |||
| if getattr(obj, '__no_type_check__', None): | |||
| return {} | |||
| if globalns is None: | |||
| globalns = getattr(obj, '__globals__', {}) | |||
| if isinstance(obj, type): | |||
| globalns = vars(sys.modules[obj.__module__]) | |||
There was a problem hiding this comment.
While you are at it maybe you can fix same way also the logic below when __mro__ is traversed
(I could imagine that base classes are from different modules). Or will it be too hard? If not, I would add also a test for such scenario.
There was a problem hiding this comment.
Yeah, I have a few ideas more but it's annoying that we don't ship ann_module.py et al. but always refer to the one in test for Python 3.5+. This makes it hard to iterate on them.
Let me see what I can do.
ambv
force-pushed
the
globalnsForClasses
branch
from
9378f79 to
7ca2449
Compare
ambv
changed the title
| # name coming from different modules in the same program. | ||
| mgc_hints = {'default_a': Optional[mod_generics_cache.A], | ||
| 'default_b': Optional[mod_generics_cache.B]} | ||
| self.assertEqual(gth(mod_generics_cache), mgc_hints) |
There was a problem hiding this comment.
I will fix this bug in a separate PR when this one lands.
There was a problem hiding this comment.
I will fix this bug in a separate PR when this one lands.
OK, just don't forget about it (or open an issue if you want to do this later).
There was a problem hiding this comment.
Hi, I was looking at a different issue and noticed this test code behaving non-deterministically depending on when mod_generics_cache was loaded (specifically this test would expectedly fail when I just ran the test_typing module, but unexpectedly succeed when I ran ./python -m test.)
Fixing the underlying type hint caching is beyond my scope as a new contributor, but it seems like something like this would make this test behave more reliably, if that seems reasonable to y'all: natgaertner/cpython@0cde103
This makes the default behavior (without specifying `globalns` manually) more predictable for users. Implementation for classes assumes has a `__module__` attribute and that module is present in `sys.modules`. It does this recursively for all bases in the MRO. For modules, the implementation just uses their `__dict__` directly. This is backwards compatible, will just raise fewer exceptions in naive user code.
ambv
force-pushed
the
globalnsForClasses
branch
from
7ca2449 to
8581ec2
Compare
| @@ -1,14 +1,53 @@ | |||
| """Module for testing the behavior of generics across different modules.""" | |||
There was a problem hiding this comment.
I only added a bunch of variable annotations here but since this wasn't compatible with < 3.5, I had to uglify this module. Now it's actually easier to see what's going on just by looking at the entire file with the "View" button.
| # name coming from different modules in the same program. | ||
| mgc_hints = {'default_a': Optional[mod_generics_cache.A], | ||
| 'default_b': Optional[mod_generics_cache.B]} | ||
| self.assertEqual(gth(mod_generics_cache), mgc_hints) |
There was a problem hiding this comment.
I will fix this bug in a separate PR when this one lands.
OK, just don't forget about it (or open an issue if you want to do this later).
|
Also, will this go into 3.6.3? |
|
Yes, that's the point. I'm working on the backport as you read this. |
|
Hopefully I'll get to fix the invalid cache of forward refs, too, before 2017-09-18 (PEP 494 3.6.3 RC). |
…ules This makes the default behavior (without specifying `globalns` manually) more predictable for users, finds the right globalns automatically. Implementation for classes assumes has a `__module__` attribute and that module is present in `sys.modules`. It does this recursively for all bases in the MRO. For modules, the implementation just uses their `__dict__` directly. This is backwards compatible, will just raise fewer exceptions in naive user code. Originally implemented and reviewed at python/typing#470.
Yes, it would be great if this gets in too. |
…nd modules (pythonGH-3582) This makes the default behavior (without specifying `globalns` manually) more predictable for users, finds the right globalns automatically. Implementation for classes assumes has a `__module__` attribute and that module is present in `sys.modules`. It does this recursively for all bases in the MRO. For modules, the implementation just uses their `__dict__` directly. This is backwards compatible, will just raise fewer exceptions in naive user code. Originally implemented and reviewed at python/typing#470. (cherry picked from commit f350a26)
…ules (#3582) This makes the default behavior (without specifying `globalns` manually) more predictable for users, finds the right globalns automatically. Implementation for classes assumes has a `__module__` attribute and that module is present in `sys.modules`. It does this recursively for all bases in the MRO. For modules, the implementation just uses their `__dict__` directly. This is backwards compatible, will just raise fewer exceptions in naive user code. Originally implemented and reviewed at python/typing#470.
…nd modules (GH-3582) (#3583) This makes the default behavior (without specifying `globalns` manually) more predictable for users, finds the right globalns automatically. Implementation for classes assumes has a `__module__` attribute and that module is present in `sys.modules`. It does this recursively for all bases in the MRO. For modules, the implementation just uses their `__dict__` directly. This is backwards compatible, will just raise fewer exceptions in naive user code. Originally implemented and reviewed at python/typing#470. (cherry picked from commit f350a26)
This makes the default behavior (without specifying
globalnsmanually) morepredictable for users.
Implementation for classes assumes has a
__module__attribute and that moduleis present in
sys.modules. It does this recursively for all bases in theMRO. For modules, the implementation just uses their
__dict__directly.This is backwards compatible, will just raise fewer exceptions in naive user
code.