Skip to content

Add a direct dependency on typing-extensions#1815

Open
musicinmybrain wants to merge 1 commit intofastapi:mainfrom
musicinmybrain:typing-extensions
Open

Add a direct dependency on typing-extensions#1815
musicinmybrain wants to merge 1 commit intofastapi:mainfrom
musicinmybrain:typing-extensions

Conversation

@musicinmybrain
Copy link

Since typing_extensions is directly and unconditionally imported,

from typing_extensions import deprecated

from typing_extensions import deprecated

from typing_extensions import deprecated

from typing_extensions import Self

we should not rely on an indirect dependency via pydantic.

The minimum version, 4.5.0, was chosen as the first release that included both typing_extensions.Self and typing_extensions.deprecated.

Since typing_extensions is directly and unconditionally imported, we
should not rely on an indirect dependency. The minimum version, 4.5.0,
was chosen as the first release that included both
typing_extensions.Self and typing_extensions.deprecated.
@github-actions
Copy link
Contributor

📝 Docs preview

Last commit 76915f2 at: https://ddf1c8d6.sqlmodel.pages.dev

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.

Hi, thanks for the PR!

I agree that we shouldn't rely on indirect dependencies for external libraries that we're importing directly.

We can either follow the route this PR proposes and add typing-extensions to the dependencies, or we could install it only conditionally for older versions of Python, and use typing whenever we can. If I'm not mistaken, Python 3.13+ has deprecated in typing, for Self it's Python 3.11 and upwards. So if we use _compat to import from either depending on the Python version, we'd only have to install typing-extensions for Python 3.10, 3.11 and 3.12.

Then again, we'd gain very little as typing-extensions would still be installed by pydantic on any Python version 🤷. So maybe the solution in this PR is just the easiest one.

@YuriiMotov: do you have an opinion?

I'll tentavily approve this in the meantime 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants