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

Move model before validators #5898

Merged
merged 4 commits into from
May 28, 2023
Merged

Conversation

samuelcolvin
Copy link
Member

@samuelcolvin samuelcolvin commented May 27, 2023

As discussed yesterday with @dmontagu.

I know this changes the behaviour of test_model_validator_before, but I think it's actually more correct.

Previously before model validators were called when the input is a model, even when validation wasn't being run. Now before validators are called in the same scenarios as the rest of model validation is run:

  • on both instances of the model type and instances of subclasses if revalidate_instances='always'
  • only on instances of subclasses if revalidate_instances='subclass-instances'
  • on neither instances of the model type nor instances of subclasses if revalidate_instances='never'

As previously, before model validators are always called when the input does not pass isinstance(input, Model).

This change does mean the logic for when before validators are called differs from wrap and after model validators - but honestly I think wrap and after model validators are wrong, although I'm not sure that's worth changing.

We should probably add tests for:

  • model_validator(mode='before') on RootModel
  • model_validator(mode='before') on dataclass

Selected Reviewer: @adriangb

@cloudflare-pages
Copy link

cloudflare-pages bot commented May 27, 2023

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 652ed99
Status: ✅  Deploy successful!
Preview URL: https://92032b70.pydantic-docs2.pages.dev
Branch Preview URL: https://move-model-before-validators.pydantic-docs2.pages.dev

View logs

Copy link
Member

@adriangb adriangb left a comment

Choose a reason for hiding this comment

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

What's the change of behavior for test_model_validator_before? I don't see that test being changed.

pydantic/_internal/_generate_schema.py Outdated Show resolved Hide resolved
tests/test_model_validator.py Show resolved Hide resolved
@samuelcolvin
Copy link
Member Author

please review.

@samuelcolvin samuelcolvin merged commit fffea02 into main May 28, 2023
53 checks passed
@samuelcolvin samuelcolvin deleted the move-model-before-validators branch May 28, 2023 12:23
@samuelcolvin
Copy link
Member Author

Merging this, happy to change/revert/add more if needed.

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

Successfully merging this pull request may close these issues.

None yet

2 participants