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

Use typing_extensions only when needed #5331

Merged
merged 3 commits into from
Jun 24, 2024

Conversation

qarkai
Copy link
Contributor

@qarkai qarkai commented Jun 22, 2024

Description

Use typing_extensions only for Python <= 3.10.

Self was added in Python 3.11.
TypeAlias was added in Python 3.10.

To Do

  • Documentation.
  • Changelog.
  • Tests.

qarkai added 2 commits June 22, 2024 16:02
Self was added in Python 3.11
TypeAlias was added in Python 3.10
@bal-e
Copy link
Member

bal-e commented Jun 22, 2024

Since type annotations are only used by mypy, maybe we should make these statements conditional on typing.TYPE_CHECKING as well? In any case, I think an explicit if statement (e.g. on the running Python version) would be preferable over catching an ImportError.

(Disclaimer: I'm not a maintainer, I'm just providing some feedback.)

@qarkai
Copy link
Contributor Author

qarkai commented Jun 22, 2024

Ruff outputs runtime-import-in-type-checking-block (TCH004) warning if from typing_extensions import is in type-checking block. Not sure why.

@bal-e
Copy link
Member

bal-e commented Jun 22, 2024

I suspect Ruff special-cases imports in type-checking blocks, only allowing things from modules it knows about. It definitely looks like they support some imports in type-checking blocks: it's mentioned here for example.

@Serene-Arc
Copy link
Contributor

Hi! Thanks for the PR. What is the benefit of doing this rather than what we already have? Python 3.8 is currently our minimum supported version.

@qarkai
Copy link
Contributor Author

qarkai commented Jun 23, 2024

Hi! Thanks for the PR. What is the benefit of doing this rather than what we already have? Python 3.8 is currently our minimum supported version.

Use built-in module in modern Python.
Reduce dependencies, make life easier for downstream maintainers.
Simplify upgrading to newer Python versions in the future, i.e. when minimum supported version is Python 3.11 typing_extensions could be removed completely.

@Serene-Arc
Copy link
Contributor

Those benefits would only happen when we have Python 3.10 as the minimally supported version version though, wouldn't it? There'd be no difference until we reach that point, since it will still be on the dependency list regardless.

@qarkai
Copy link
Contributor Author

qarkai commented Jun 23, 2024

Nope, if distro has Python >= 3.11 maintainer would be able to drop typing_extensions dependency for the next beets release with this patch even if the minimal supported version is still 3.8.
It's in typing_extensions = { version = "*", python = "<=3.10" } line.

Copy link
Contributor

@Serene-Arc Serene-Arc left a comment

Choose a reason for hiding this comment

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

Very well then, thanks for explaining.

@Serene-Arc Serene-Arc merged commit f1c643d into beetbox:master Jun 24, 2024
12 checks passed
@qarkai qarkai deleted the typing_extensions branch June 24, 2024 06:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants