-
Notifications
You must be signed in to change notification settings - Fork 14
Test Python 3.9 and 3.10 on CI; Apply fixes #372
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
Conversation
if sys.version_info >= (3, 10): | ||
from typing import TypeAlias | ||
else: | ||
from typing_extensions import TypeAlias |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could safely do directly from typing_extensions import TypeAlias
to avoid checking the python version twice (because typing_extensions` does it as well https://github.com/python/typing_extensions/blob/7def253cd65b3a916fcb76a6eb51428b60e1bcc8/src/typing_extensions.py#L1500-L1517
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But people on latest Python versions might not have typing_extensions installed. (we should probably mark it as a runtime dependency ourselves for earlier Python versions I guess)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(we should probably mark it as a runtime dependency ourselves for earlier Python versions I guess)
Yes we should 😅 (I didn't check the dependencies)
pydantic adds it for all version https://github.com/pydantic/pydantic/blob/main/pyproject.toml#L46
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI it didn't fail on CI because we have a lot of tests dependencies that depends on typing-extensions https://github.com/developmentseed/obstore/blob/main/uv.lock#L281
IMO it doesn't hurt that much to add typing-extensions as a dependency (except that you won't be able to say that obstore is a zero dependency module 😬)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Closes #373