Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Support parametrization of class-scoped type variables when using@validate_call on Pydantic models methods#10367

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed
kc0506 wants to merge37 commits intopydantic:mainfromkc0506:validate-call-generic

Conversation

kc0506
Copy link
Contributor

@kc0506kc0506 commentedSep 10, 2024
edited
Loading

Change Summary

The primary change of this PR is to allow code as below to work properly:

classA[T](BaseModel):@validate_call(validate_return=True)deff(self,x:T)->T:returnxa=A[int]()asserta.f(1)==1asserta.f('1')==1a.f('abc')# throw error correctly

Limitations

I classify generic functions into three cases:

1. The TypeVar is "free", or attached to a function:

T=TypeVar("T")deff(a:T): ...# ordeff[T](a:T): ...

2. The TypeVar is attached to a class that is not a subclass ofBaseModel:

classA(Generic[T]):deff(a:T): ...# orclassA[T]:deff(a:T): ...

3. The TypeVar is attached to a class thatis a subclass ofBaseModel:

classA(BaseModel,Generic[T]):deff(a:T): ...# orclassA[T](BaseModel):deff(a:T): ...

For the case 1 and 2, there is (AFAIK) unfortunately no way to access the value ofT at runtime without custom__class_getitem__. See theissue anddiscussion. Therefore there is no way to "trigger" the schema rebuilding process.

For case 3, the value ofT is always determined and can be accessed at runtime. Hence, this PR currently only supports such case.

Update: I realize that in case 1, we can override the__getitem__ such that the user can do something likef[int](...) to manually trigger rebuilding. The only problem is thatFunctionType is not allowed to be subclassed, and instance-level method override does not work for dunder methods, so we must return an object whose type is notFunctionType anymore. This seems to conflict with current design.

As for case 2, perhaps we could expose another API, e.g.enable_generic_validate_call, so that when a class is decorated by it, we override its__class_getitem__ and do the similar stuff asBaseModel.

Related issue number

#7796 (partially fixed)

Checklist

  • The pull request title is a good summary of the changes - it will be used in the changelog
  • Unit tests for the changes exist
  • Tests pass on CI
  • Documentation reflects the changes where applicable
  • My PR is ready to review,please add a comment including the phrase "please review" to assign reviewers

@github-actionsgithub-actionsbot added the relnotes-fixUsed for bugfixes. labelSep 10, 2024
@codspeed-hqCodSpeed HQ
Copy link

codspeed-hqbot commentedSep 10, 2024
edited
Loading

CodSpeed Performance Report

Merging#10367 willnot alter performance

Comparingkc0506:validate-call-generic (3dee09b) withmain (7bb0e0a)

Summary

✅ 43 untouched benchmarks

@sydney-runkle
Copy link
Contributor

I've enabled auto merge on#10100 :)

kc0506 reacted with hooray emoji

@sydney-runkle
Copy link
Contributor

I have also fixed some issues relevant to namespace errors, e.g.#10150.

Could you by chance separate this fix out into a separate PR? Makes things easier to review and easier to revert, if necessary.

@sydney-runkle
Copy link
Contributor

cc@dmontagu, perhaps you'd want to take a first pass review on this one?

@kc0506
Copy link
ContributorAuthor

Could you by chance separate this fix out into a separate PR? Makes things easier to review and easier to revert, if necessary.

Certainly, check#10380.

sydney-runkle reacted with heart emoji

@kc0506kc0506force-pushed thevalidate-call-generic branch 2 times, most recently from87d62f4 to23f7aa6CompareSeptember 11, 2024 14:22
@github-actionsGitHub Actions
Copy link
Contributor

github-actionsbot commentedSep 11, 2024
edited
Loading

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  pydantic
  main.py
  validate_call_decorator.py
  pydantic/_internal
  _generics.py
  _model_construction.py
  _validate_call.py174,190
Project Total 

This report was generated bypython-coverage-comment-action

Copy link
Contributor

@dmontagudmontagu left a comment

Choose a reason for hiding this comment

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

This seems like a very smart implementation, and I think it's a good piece of functionality to get working, but I'd really prefer if there was any way we could achieve it without usingexec and class generation codegen; something just seems very fragile and/or security-risky about that.

@kc0506
Copy link
ContributorAuthor

kc0506 commentedSep 12, 2024
edited
Loading

I have completely removed the hackyexec code. It turns not that was unnecessary.

Update: I realize that in case 1, we can override the__getitem__ such that the user can do something likef[int](...) to manually trigger rebuilding. The only problem is thatFunctionType is not allowed to be subclassed, and instance-level method override does not work for dunder methods, so we must return an object whose type is notFunctionType anymore. This seems to conflict with current design.

@dmontagu btw, what do you think about this workaround? From the current design, it seems we want to return a normal function instead of a wrapper class bypydantic. Is the workaround a good reason to change that design? This probably belongs to another PR tho.

@ViicosViicos self-requested a reviewSeptember 12, 2024 12:01
@ViicosViicos changed the titleImprovevalidate_call with generic functionSupport parametrization of class-scoped type variables when using@validate_call on Pydantic models methodsSep 12, 2024
function.__qualname__ = qualname.replace(original_postfix, f'{model.__name__}.{name}')


def update_generic_validate_calls(model: type[BaseModel]) -> None:
Copy link
Member

Choose a reason for hiding this comment

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

You will have to add atry..finally block in case any exception happens, as otherwise the original function will have its attributes mutated.

Also, I'll have to think about it more, but are we certain that every callable that can be used withvalidate_call can have the__qualname__/__annotations__ etc arguments mutated?

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Ah, I think I forgot to consider callables with various type in many places. Later I should add more tests withpartial,wraps etc.

Also,#10461 should probably be dealt with first to better recognize what types of functions we may have here.

@kc0506kc0506 mentioned this pull requestOct 18, 2024
13 tasks
@Viicos
Copy link
Member

We are going to close this for now, as you mentioned it is difficult to properly support it and there isn't enough upvotes on the issue to be considered worthwhile. Thanks again for taking a stab at this, we can revisit if we ever want to implement it.

@ViicosViicos closed thisFeb 12, 2025
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@dmontagudmontagudmontagu left review comments

@ViicosViicosViicos requested changes

Assignees
No one assigned
Labels
relnotes-fixUsed for bugfixes.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

4 participants
@kc0506@sydney-runkle@Viicos@dmontagu

[8]ページ先頭

©2009-2025 Movatter.jp