Skip to content

WIP: refactor annotation injection for known (often generic) types#9979

Merged
sydney-runkle merged 10 commits intomainfrom
change-match-order
Jul 29, 2024
Merged

WIP: refactor annotation injection for known (often generic) types#9979
sydney-runkle merged 10 commits intomainfrom
change-match-order

Conversation

@sydney-runkle
Copy link
Copy Markdown
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
Copy Markdown

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
Copy Markdown

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
Copy Markdown
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
Copy Markdown
Contributor Author

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

@adriangb
Copy link
Copy Markdown
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.

2 participants