change convention for typing_extensions#15870
Merged
Merged
Conversation
Signed-off-by: nstarman <[email protected]>
Signed-off-by: nstarman <[email protected]>
Contributor
|
Thank you for your contribution to Astropy! 🌌 This checklist is meant to remind the package maintainers who will review this pull request of some common things to look for.
|
eerovaher
approved these changes
Jan 12, 2024
Member
eerovaher
left a comment
eerovaher
left a comment
There was a problem hiding this comment.
The only thing that prevents importing from typing_extensions without the explicit version check is maintainer vigilance. Our CI doesn't help because if TYPE_CHECKING: blocks are not executed at runtime. These version checks are not something we should be burdening maintainers with.
Contributor
|
OK, two approvals, so let's get it in. Thanks, @nstarman! |
2 tasks
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See #15867 for discussion!
In a recent #15686 it was noted that typing_extensions is not a runtime dependency and as is consistent with our current policy we added a version check. That was good!
Now that we're starting to add type annotations, and typing_extensions is an integral part of that effort I think it's worth formalizing how we deal with typing_extensions.
typing_extensions is developed within the core Python org (https://github.com/python/typing_extensions). It is effectively core python. Its mission is to provide backports of typing for use in earlier versions of Python. Internally typing_extensions does Python version checks to determine whether it should just re-export from typing or needs to define the official backport. ALL standard type checkers, like mypy, recognize typing_extensions since it is "core" python. Moreover, ruff has specific rules to automatically change our imports from typing_extensions to typing as we bump our minimum python version.
Putting all this together. Let's make our lives easier and reduce code clutter by trusting typing_extensions and our CI. We don't need to do manual Python version checks.
Let's make this our policy for typing_extensions!
Q: Why not make typing_extensions a runtime dependency?
Because we don't yet need it at runtime.
Q: is it really a lot of code clutter?
Heck yeah. This PR has one example. See also #15860 and #15853 (comment).
I suspect most files in Astropy will have this avoidable clutter.