8000 WIP: refactor annotation injection for known (often generic) types by sydney-runkle · Pull Request #9979 · pydantic/pydantic · GitHub
[go: up one dir, main page]

Skip to content

Conversation

@sydney-runkle
Copy link
Contributor
@sydney-runkle sydney-runkle commented Jul 25, 2024

Main accomplishments of this PR, which are easiest to review on a commit by commit basis.

  • Adding a new test directive for testing w o docs tests, bc docs tests are slow
  • Remove unused config arg for the remaining annotation prep functions
  • we already handle the sequence case directly for generic sequences, so do the same for non parametrized ones
  • address specific cases with effectively a switch statement in the prep annotations func to avoid unnecessary function calls
  • move the get_origin check before the annotations prep to ensure we use a more simple path for types like list[str]

Seeing some performance improvements on the _apply_annotations front :). Looks like a 2.8x speedup in the annotation prep function for known types.

I'm looking for feedback on the above, as well as the fact that there are now 5 failing tests related to the fact that we now fail to raise annotation application errors, similar to #9977 (comment).

I would like to address this in a separate PR as mentioned in my comment above. If you agree @adrian, I can mark these tests as xfail as well.

@github-actions github-actions bot added the relnotes-change Used for changes to existing functionality which don't have a better categorization. label Jul 25, 2024
@cloudflare-workers-and-pages
Copy link
cloudflare-workers-and-pages bot commented Jul 25, 2024

Deploying pydantic-docs with  Cloudflare Pages  Cloudflare Pages

Latest commit: 228ed5b
Status: ✅  Deploy successful!
Preview URL: https://67546565.pydantic-docs.pages.dev
Branch Preview URL: https://change-match-order.pydantic-docs.pages.dev

View logs

@codspeed-hq
Copy link
codspeed-hq bot commented Jul 25, 2024

CodSpeed Performance Report

Merging #9979 will not alter performance

Comparing change-match-order (228ed5b) with main (2d37b66)

Summary

✅ 14 untouched benchmarks

@github-actions
Copy link
Contributor
github-actions bot commented Jul 25, 2024

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  pydantic/_internal
  _generate_schema.py
  _std_types_schema.py
Project Total  

This report was generated by python-coverage-comment-action

@sydney-runkle
Copy link
Contributor Author

If approved, I can mark these as xfails and attach a github issue to be addressed pre v2.9 :)

@adriangb
Copy link
Member

Looks good to me but tests are failing. Please fix those before we can merge.

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

Labels

relnotes-change Used for changes to existing functionality which don't have a better categorization.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

0