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

Move core schema generation logic for path types inside theGenerateSchema class#10846

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

Merged
sydney-runkle merged 11 commits intomainfromperf-remove-prep-anns
Jan 2, 2025

Conversation

sydney-runkle
Copy link
Contributor

@sydney-runklesydney-runkle commentedNov 14, 2024
edited
Loading

Some (verbose) context:#10036

I'm trying to remove the use ofprepare_pydantic_annotations_for_known_type internally. We call this function in order to to pull metadata off of types and inject said metadata's constraints into core schema. This is bad practice, as we generally advertise being religious about respecting metadata order when building schema, but we violate that principle here.

Though these checks don't drag much on performance, they do significantly clutter our internal schema generation logic. They represent the old pattern (we have since deprecated) of supporting a__pydantic_prepare_annotations__ method.

For example, we used to pull string constraint data likemin_length off of aPath like annotation + apply that to the innercore_schema.str_schema() orcore_schema.bytes_schema().

This removes support for constraints likemax_length,min_length, andstrip_whitespace or other string constraints onPath types. Users should implement these constraints with custom validators.This should be advertised in the v2.11 changelog.

@github-actionsgithub-actionsbot added the relnotes-fixUsed for bugfixes. labelNov 14, 2024
@cloudflare-workers-and-pagesCloudflare Workers and Pages
Copy link

cloudflare-workers-and-pagesbot commentedNov 14, 2024
edited
Loading

Deploying pydantic-docs with  Cloudflare Pages  Cloudflare Pages

Latest commit:b2b8f7e
Status: ✅  Deploy successful!
Preview URL:https://b439c509.pydantic-docs.pages.dev
Branch Preview URL:https://perf-remove-prep-anns.pydantic-docs.pages.dev

View logs

@codspeed-hqCodSpeed HQ
Copy link

codspeed-hqbot commentedNov 14, 2024
edited
Loading

CodSpeed Performance Report

Merging#10846 willnot alter performance

Comparingperf-remove-prep-anns (b2b8f7e) withmain (9553fa4)

Summary

✅ 46 untouched benchmarks

@sydney-runklesydney-runkle changed the titleWIP: removing prepare pydantic annotations logicRemoving prepare pydantic annotations (for known types) logicNov 14, 2024
@github-actionsGitHub Actions
Copy link
Contributor

github-actionsbot commentedNov 14, 2024
edited
Loading

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  pydantic/_internal
  _generate_schema.py485,498-501,503,506-507
  _std_types_schema.py
Project Total 

This report was generated bypython-coverage-comment-action

Copy link
Member

@ViicosViicos left a comment

Choose a reason for hiding this comment

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

What does this tell us about our annotation application in general?

I think it reveals a limitation of our current annotation application. Because paths could be considered as a custom type (i.e. there's no core schema that represents this type, we currently build the validation logic with a customafter validator), we face the limitation of not having access to the annotated metadata as describedhere.

The currentmin/max_length_validator function is not going to scale well in the future if we ever want to add support formin/max_length for another type similar toPath (and that type similarly doesn't have a core schema type for it).

I'll note that while this limitation shows up here with a type without a proper core schema for it, we also face weird behaviors with the following (and unlikePath,int has a core schema type):

defval(v):print('val called')returnvclassModel(BaseModel):a:Annotated[int,AfterValidator(val),Field(gt=0)]Model(a=-1)#> val called#> ValidationError raised
  • the after validator is calledbefore thegt validation, which imo is not what should happen.
  • as a consequence, thegt constraint isn't represented in theint_schema(), but instead thegreater_than_validator Python fallback validator function is used.

On another note, maybe we should discuss how we validate "concrete" vs "abstract" types. For instance, for paths, we usepathlib.PurePath foros.PathLike annotations, but this is an opinionated choice and users could expectPath to be used instead.

This relates to the behavior ofIterable: we currently validate these by creating aValidationIterator instance, and people complained about it. Maybe we should have a way of customizing which "concrete" implementation should be used for "abstract" types?

sydney-runkle reacted with thumbs up emoji
Co-authored-by: Victorien <65306057+Viicos@users.noreply.github.com>
@sydney-runklesydney-runkle added relnotes-changeUsed for changes to existing functionality which don't have a better categorization. and removed relnotes-fixUsed for bugfixes. labelsNov 21, 2024
@sydney-runklesydney-runkle changed the titleRemoving prepare pydantic annotations (for known types) logicRemoving prepare pydantic annotations (for known types) logic -Path typesNov 21, 2024
@sydney-runklesydney-runkle requested review fromViicos and removed request foradriangbJanuary 2, 2025 15:53
@sydney-runklesydney-runkle added the third-party-testsAdd this label on a PR to trigger 3rd party tests labelJan 2, 2025
@ViicosViicos changed the titleRemoving prepare pydantic annotations (for known types) logic -Path typesMove core schema generation logic for path types inside theGenerateSchema classJan 2, 2025
@sydney-runklesydney-runkle merged commitaa85c3a intomainJan 2, 2025
89 checks passed
@sydney-runklesydney-runkle deleted the perf-remove-prep-anns branchJanuary 2, 2025 17:25
recursiveAF pushed a commit to recursiveAF/pydantic that referenced this pull requestFeb 22, 2025
…Schema` class (pydantic#10846)Co-authored-by: Victorien <65306057+Viicos@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@adriangbadriangbadriangb left review comments

@ViicosViicosViicos approved these changes

Assignees
No one assigned
Labels
relnotes-changeUsed for changes to existing functionality which don't have a better categorization.third-party-testsAdd this label on a PR to trigger 3rd party tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

3 participants
@sydney-runkle@adriangb@Viicos

[8]ページ先頭

©2009-2025 Movatter.jp