Skip to content

Use identity instead of equality after validating model in __init__#10264

Merged
sydney-runkle merged 2 commits intomainfrom
init-fix
Aug 29, 2024
Merged

Use identity instead of equality after validating model in __init__#10264
sydney-runkle merged 2 commits intomainfrom
init-fix

Conversation

@Viicos
Copy link
Copy Markdown
Member

@Viicos Viicos commented Aug 29, 2024

Change Summary

Follow up on #10255.

This is more accurate and avoids the big performant hit.

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-actions github-actions Bot added the relnotes-fix Used for bugfixes. label Aug 29, 2024
@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented Aug 29, 2024

Deploying pydantic-docs with  Cloudflare Pages  Cloudflare Pages

Latest commit: da25e53
Status: ✅  Deploy successful!
Preview URL: https://1fbd0bb5.pydantic-docs.pages.dev
Branch Preview URL: https://init-fix.pydantic-docs.pages.dev

View logs

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Aug 29, 2024

CodSpeed Performance Report

Merging #10264 will improve performances by 36.56%

Comparing init-fix (da25e53) with main (e13b15e)

Summary

⚡ 4 improvements
✅ 30 untouched benchmarks

Benchmarks breakdown

Benchmark main init-fix Change
test_complex_model_validation[__init__] 75.7 µs 57.6 µs +31.27%
test_list_of_models_validation[__init__] 115.7 µs 99.9 µs +15.81%
test_nested_model_validation[__init__] 75.1 µs 60.2 µs +24.78%
test_simple_model_validation[__init__] 58.9 µs 43.2 µs +36.56%

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Aug 29, 2024

Coverage report

This PR does not seem to contain any modification to coverable code.

Copy link
Copy Markdown
Contributor

@sydney-runkle sydney-runkle left a comment

Choose a reason for hiding this comment

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

Change looks good, thanks

Comment thread tests/test_validators.py Outdated
Comment on lines +2921 to +2924
return Child.model_construct(name='different!')
return Child.model_construct(name='name')

with pytest.warns(UserWarning, match='A custom validator is returning a value other than `self`'):
Child(name='foo')
Child(name='name')
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd rather leave these as different to illustrate the odd behavior...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

To make it more clear, we could add an assert and a comment at the end (like self overrides, blah blah blah)

@pydantic-hooky pydantic-hooky Bot added the awaiting author revision awaiting changes from the PR author label Aug 29, 2024
@sydney-runkle
Copy link
Copy Markdown
Contributor

Great work on the perf boost here.

@sydney-runkle sydney-runkle enabled auto-merge (squash) August 29, 2024 19:17
@sydney-runkle sydney-runkle merged commit f9f6ea6 into main Aug 29, 2024
@sydney-runkle sydney-runkle deleted the init-fix branch August 29, 2024 19:29
AdolfoVillalobos pushed a commit to AdolfoVillalobos/pydantic that referenced this pull request Aug 30, 2024
…pydantic#10264)

Co-authored-by: sydney-runkle <sydneymarierunkle@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting author revision awaiting changes from the PR author relnotes-fix Used for bugfixes.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants