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

Fix strict application to function-after with use_enum_values #9279

Merged
merged 7 commits into from
Apr 19, 2024

Conversation

sydney-runkle
Copy link
Member

@sydney-runkle sydney-runkle commented Apr 18, 2024

Fix #9257

Selected Reviewer: @samuelcolvin

@sydney-runkle
Copy link
Member Author

Please review @dmontagu

Copy link

cloudflare-workers-and-pages bot commented Apr 18, 2024

Deploying pydantic-docs with  Cloudflare Pages  Cloudflare Pages

Latest commit: 9f375c4
Status: ✅  Deploy successful!
Preview URL: https://53f6dc51.pydantic-docs2.pages.dev
Branch Preview URL: https://fix-enum-strict-schema.pydantic-docs2.pages.dev

View logs

Copy link

codspeed-hq bot commented Apr 18, 2024

CodSpeed Performance Report

Merging #9279 will not alter performance

Comparing fix_enum_strict_schema (9f375c4) with main (3c15a8b)

Summary

✅ 13 untouched benchmarks

@@ -195,6 195,12 @@ def apply_known_metadata(annotation: Any, schema: CoreSchema) -> CoreSchema | No
raise ValueError(f'Unknown constraint {constraint}')
allowed_schemas = CONSTRAINTS_TO_ALLOWED_SCHEMAS[constraint]

# if it becomes necessary to handle more than one constraint
# in this recursive case with function-after or function-wrap, we should refactor
if schema_type in ['function-before', 'function-wrap', 'function-after'] and constraint == 'strict':
Copy link
Contributor

Choose a reason for hiding this comment

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

Better to use a set here, and possibly store it out of line. I’d be okay with just making it a set though

Copy link
Contributor

@dmontagu dmontagu left a comment

Choose a reason for hiding this comment

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

As noted in #9279, there are a bunch of cases related to strict still not handled properly, that might be handled properly through a very similar change. But we can deal with them later.

@sydney-runkle sydney-runkle added this to the v2.8.0 milestone Apr 18, 2024
@sydney-runkle
Copy link
Member Author

Yep. Added said issue to the 2.8 milestone!

@sydney-runkle sydney-runkle enabled auto-merge (squash) April 19, 2024 00:45
@sydney-runkle sydney-runkle removed this from the v2.8.0 milestone Apr 19, 2024
@sydney-runkle
Copy link
Member Author

Here's that issue: #9281

@sydney-runkle sydney-runkle merged commit 6322b24 into main Apr 19, 2024
53 checks passed
@sydney-runkle sydney-runkle deleted the fix_enum_strict_schema branch April 19, 2024 00:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error: RuntimeError: Unable to apply constraint strict to schema function-after
3 participants