Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Initial guidance on using type annotations #49

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

joachimmetz
Copy link
Member

No description provided.

process/Style-guide.md Outdated Show resolved Hide resolved

* Do not use `from __future__ import annotations` since it is not compatible with Python 3.6
* For importing typing see: http://google.github.io/styleguide/pyguide.html#31912-imports-for-typing
* Define type annotations as strings so `'int'` instead of `int`. This prevents having issues where types are self referencing
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure it's worth adding all these characters for the small number of classes that reference themselves. Let's just do this on the rare cases it's necessary.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So http://google.github.io/styleguide/pyguide.html#31912-imports-for-typing states:

Conditionally imported types need to be referenced as strings, to be forward compatible with Python 3.6 where the annotation expressions are actually evaluated.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recommend us sticking with strings for now to be consistent

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about instead we wait until we've dropped support for python 3.6, as this string ugliness isn't required in Python 3.7+ https://www.python.org/dev/peps/pep-0563/

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So type annotations will remain somewhat "ugly" with Python 3.7 and later as well, but we can postpone adopting them once we are able to drop 3.6 support.

@joachimmetz joachimmetz requested a review from Onager June 5, 2020 11:41
@joachimmetz joachimmetz added the blocked Work cannot progress until another issue is resolved label Jun 5, 2020
@joachimmetz joachimmetz marked this pull request as draft June 5, 2020 11:41
Base automatically changed from master to main February 8, 2021 07:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked Work cannot progress until another issue is resolved
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants