Use get_typing_hints instead of __annotations__ to resolve types in Python 3.10 #297
Conversation
facebook-github-bot
added
the
CLA Signed
|
Hey @tirkarthi , Thanks for the PR, please |
|
Thanks @deathowl I have fixed the formatting issues. I will wait if someone has issues on using get_type_hints by default to emulate behavior before Python 3.10 and https://www.python.org/dev/peps/pep-0563/#backwards-compatibility |
Pull Request Test Coverage Report for Build 748441995
💛 - Coveralls |
|
@deathowl has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
testslide/strict_mock.py
Outdated
| if hasattr(self._template, "__annotations__"): | ||
| annotations = self._template.__annotations__ | ||
| if self._template is not None: | ||
| annotations = get_type_hints(self._template) |
There was a problem hiding this comment.
This does not play nice with pybind11 based modules
in get_type_hints
base_globals = sys.modules[base.__module__].__dict__
KeyError: 'pybind11_builtins'
There was a problem hiding this comment.
Thanks, this seems to have caused an issue in sphinx as well where they catch keyerror and have empty annotations. I don't have the right setup but it will be good to know what was the value of annotations using __annotations__ attribute in earlier versions.
There was a problem hiding this comment.
I guess catching key error and returning empty annotations would work well as a workaround for us as well
There was a problem hiding this comment.
Thanks, I will make the changes. On further search seems this is an open issue in CPython where there is a PR open to catch the KeyError and discard it for these type of cases : https://bugs.python.org/issue41515
|
@deathowl has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
testslide/strict_mock.py
Outdated
| annotations = get_type_hints(self._template) | ||
| try: | ||
| annotations = get_type_hints(self._template) | ||
| except KeyError: |
There was a problem hiding this comment.
can we do except Exception just to make sure we dont break tests when we fail to get typehints for any funny modules?
There was a problem hiding this comment.
KeyError seemed more specific to the modules with different __module__ . Are there any other failures that expose a bug with get_type_hints? I couldn't see the test failures since it's private.
There was a problem hiding this comment.
https://www.python.org/dev/peps/pep-0563/#runtime-annotation-resolution-and-class-decorators it can throw NameError on metaclasses
|
@deathowl has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
What:
Fixes #296
Why:
Due to PEP 563 becoming default in Python 3.10 the
__annotations__value doesn't store types and it stores string values. Usingtyping.get_type_hintsensures the types are evaluated and returned like behavior before Python 3.10 .How:
Risks:
Checklist: