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.

Sign up for GitHub

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

Jump to bottom

MAINT: update all TypedDict imports [skip ci] #25352

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

MAINT: update all TypedDict imports [skip ci] #25352

wants to merge 1 commit into from

Conversation

rbavery
Copy link

@rbavery rbavery commented Dec 9, 2023 •
edited
Loading

addresses #25206 by updating where TypedDict is imported from. This is to address an error pydantic raises when numpy_typing.DTypeLike is used in pydantic v2 BaseModels.

I'm skipping ci since this is my first PR and I'm not sure if I should skip some of the CI described here given that the change is minimal: https://numpy.org/devdocs/dev/development_workflow.html#commands-to-skip-continuous-integration

@rbavery rbavery mentioned this pull request Dec 9, 2023
Open
rgommers
rgommers requested changes Dec 11, 2023
View reviewed changes
Copy link
Member

@rgommers rgommers left a comment

Choose a reason for hiding this comment

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

Thanks @rbavery. The changes to .pyi files look fine to me, but the one to the _typing/_dtype_like.py does not. That adds a runtime dependency on typing_extensions, which must not be done. typing_extensions can only be used at static analysis time. So that import must either be avoided or protected by if typing.TYPE_CHECKING:.

@BvB93
Copy link
Member

BvB93 commented Dec 11, 2023

For the issue in particular the *.pyi changes are completely redundant anyway as Pydanticis purely a runtime tool; not a static type checker. The only way to fix this issue would be via the runtime import of from typing_extensions import TypedDict (in combination with a try/except block), reintroduction of which I'm admittedly lukewarm about. Especially considering, as was already noted by Ralf, this issue is due to a (overly?) strict typeddict requirement on Pydantics side rather than a numpy bug.

Now, if we're going ahead with the runtime import from typing extensions then we could also really use a reintroduction of this test (xref #19525) in order to assure we don't accidentally make typing extensions a hard dependency.

@rbavery
Copy link
Author

rbavery commented Dec 12, 2023

Thanks for the feedback!

The only way to fix this issue would be via the runtime import of from typing_extensions import TypedDict (in combination with a try/except block), reintroduction of which I'm admittedly lukewarm about.

I'll leave it up to the numpy maintainers if this is something that is alright to do. I agree the requirement is overly strict. Unfortunately I don't think pydantic will adjust this behavior since they closed issues related to this pretty emphatically. I don't need to use npt.DtypeLike in pydantic BaseModels but it would be nice if this worked.

pydantic/pydantic#6860 (comment)
pydantic/pydantic#6645 (comment)

@charris charris changed the title [MAINT] update all TypedDict imports [skip ci] MAINT: update all TypedDict imports [skip ci] Aug 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rgommers rgommers rgommers requested changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
03 - Maintenance 41 - Static typing
Projects
NumPy first-time contributor PRs
Status: Pending authors' response
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@rbavery @BvB93 @rgommers