Skip to content

Conversation

@joachimmetz
Copy link
Member

No description provided.


* 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.

3 participants