Support for string type annotations in attrs #2
Conversation
It doesn't cover all the possible cases because there are limitations in attrs itself. See python-attrs/attrs#593 Enabled py35 in Travis
Codecov Report
@@ Coverage Diff @@
## master #2 +/- ##
==========================================
+ Coverage 92.1% 94.23% +2.12%
==========================================
Files 3 3
Lines 38 52 +14
Branches 8 9 +1
==========================================
+ Hits 35 49 +14
Misses 1 1
Partials 2 2
|
andi/andi.py
Outdated
| @@ -7,11 +9,40 @@ | |||
| from andi.typeutils import get_union_args, is_union | |||
|
|
|||
|
|
|||
| def _get_globalns_as_get_type_hints(func) -> Dict: | |||
| """ Global namespace resolution extracted from ``get_type_hints`` method """ | |||
There was a problem hiding this comment.
Where is this function copied from? It'd be nice to have a link here, maybe to cpython repo on github.
Could you please also check https://codecov.io/gh/scrapinghub/andi/compare/b64bf74d4182e3a96563c4e35ec10d396d2164a4...665c6a53bb5ba511dfbfcbe25223d915c91500e8/diff? It shows that parts of this code are never called in tests.
I'm asking because locally for me the relevant code is just
if globalns is None:
globalns = getattr(obj, '__globals__', {})This is likely because it is from Python 3.6 - what Python version have you copied it from? It would be good to preserve this, as the code is changing upstream.
There was a problem hiding this comment.
I copied from 3.7: https://github.com/python/cpython/blob/3.7/Lib/typing.py#L981-L988 . I've put the link in the docstring here:3cf30d3
It shows that parts of this code are never called in tests.
I'll try to create some tests that cover these lines. I'll have to figure out what is the __wrapped__ for.
andi/andi.py
Outdated
| @@ -7,11 +9,41 @@ | |||
| from andi.typeutils import get_union_args, is_union | |||
|
|
|||
|
|
|||
| def _get_globalns_as_get_type_hints(func) -> Dict: | |||
| """ Global namespace resolution extracted from ``get_type_hints`` method. | |||
| Python 3.7 (https://github.com/python/cpython/blob/3.7/Lib/typing.py#L981-L988) """ | |||
There was a problem hiding this comment.
By the way, it is different again in Python 3.8; in this case it mentions sys.modules[func.__module__].__dict__. Would you mind checking how it works in Python 3.8 - are workarounds still needed?
There was a problem hiding this comment.
Apparently this part of the code is the same in 3.7 and 3.8. I think it is right as it is.
| ns.update(_get_globalns_as_get_type_hints(func)) | ||
| return ns | ||
|
|
||
|
|
||
| def inspect(func: Callable) -> Dict[str, List[Optional[Type]]]: |
There was a problem hiding this comment.
@kmike an alternative to fix the attrs problem would be to provide a inspect for classes. Something like:
def inspect_cls(cls: type):
...
annotations = get_type_hints(cls.__init__, cls.__globals__)
...In this case I think we would have access to the global namespace easily. What do you think?
|
Thanks @ivanprado! I think that's fine to merge it as-is. |
|
Thanks @kmike for the review! |
It doesn't cover all the possible cases because there are limitations in attrs itself. See python-attrs/attrs#593
Enabled py35 in Travis