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

✨ Avoid the unnecessary import of typing_extensions in newer Python versions #1048

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

horta
Copy link

@horta horta commented Nov 12, 2024

@svlandeg svlandeg changed the title Avoid the unnecessary import of typing_extensions ✨ Avoid the unnecessary import of typing_extensions in newer Python versions Nov 13, 2024
@svlandeg svlandeg added the feature New feature, enhancement or request label Nov 13, 2024
Copy link
Member

@svlandeg svlandeg left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! This makes sense. We do have the check in other places where we're importing things like Literal, though we're checking on v3.8 in those instances. From the link you shared, I see that typing_extensions has an updated definition for 3.8, so it makes sense to only import from typing from 3.9 onwards. Perhaps we should even make that change across the codebase to be consistent.

typer/main.py Outdated Show resolved Hide resolved
@horta
Copy link
Author

horta commented Nov 13, 2024

I dont understand that linting error for the rich_utils.py file. If I import Literal directly from typing_extensions or typing, it works, but not if I import it from _typing?

@svlandeg svlandeg self-assigned this Nov 14, 2024
@svlandeg
Copy link
Member

Hm, that's weird. I can have a look sometime later this week if you don't figure it out!

@svlandeg svlandeg marked this pull request as draft November 14, 2024 11:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature, enhancement or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants