Skip to content

Conversation

sydney-runkle
Copy link
Contributor

@sydney-runkle sydney-runkle commented Jul 30, 2024

An attempt at addressing #9988

We apply annotations in 2 ways, in order of preference:

  1. Directly to the type, like le to int
  2. As an after validator wrapper that enforces a given constraint, like le around an arbitrary function after around an int. This ensures we respect annotation order.

We try to be pretty lax with #2, but this can cause problems if even a functional validator applying the given constraint isn't compatible with the inner type. Thus, I've added some support to our builtin functional validators to raise informative error messages when applying a constraint raises a TypeError.

Fix #9988

@github-actions github-actions bot added the relnotes-fix Used for bugfixes. label Jul 30, 2024
Copy link

cloudflare-workers-and-pages bot commented Jul 30, 2024

Deploying pydantic-docs with  Cloudflare Pages  Cloudflare Pages

Latest commit: fd12aa4
Status: ✅  Deploy successful!
Preview URL: https://34e38efc.pydantic-docs.pages.dev
Branch Preview URL: https://strict-annotation-apps.pydantic-docs.pages.dev

View logs

Copy link

codspeed-hq bot commented Jul 30, 2024

CodSpeed Performance Report

Merging #9999 will not alter performance

Comparing strict-annotation-apps (fd12aa4) with main (f45033e)

Summary

✅ 14 untouched benchmarks

Copy link
Contributor

github-actions bot commented Jul 30, 2024

Coverage report

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

@sydney-runkle sydney-runkle added relnotes-change Used for changes to existing functionality which don't have a better categorization. and removed relnotes-fix Used for bugfixes. labels Jul 30, 2024
@sydney-runkle sydney-runkle requested a review from adriangb July 31, 2024 11:55
@sydney-runkle sydney-runkle changed the title Be more strict with TypeErrors for improper annotations Move annotation compatibility errors to validation phase Jul 31, 2024
Copy link
Member

@adriangb adriangb left a comment

Choose a reason for hiding this comment

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

Looks good!

@sydney-runkle
Copy link
Contributor Author

We discussed one design pattern here - the create_constraint_validator ensures consistency in terms of error messages for lots of the constraints. It's a bit harder to understand than having more verbose + simple code.

That's a tradeoff I'd be willing to debate further in the future, if folks have strong opinions, but I think this is a good step forward for now.

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.

Enforce more strict annotation checks
2 participants