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: Perform choices validation after parse/mapping time. #179

Merged
merged 1 commit into from
Nov 20, 2024

Conversation

DanCardin
Copy link
Owner

@DanCardin DanCardin commented Nov 13, 2024

Fixes #170.

Although i still want to look at this a little bit more. this does add some weird code duplication/different branches. Although I suppose some of that already existed before, it was just built into the parser.

In order to avoid needing to physically manipulate the annotations, this PR drops the nicer "choices" invalid choice: 'thename' (choose from 'one', 'two', 'three', 4)-style error messages for annotations like Union[Literal['one'], Literal['two']], because it becomes impractical to support.

Instead such annotations get the union-like error message

invalid value for 'name': possible variants
 - literal['one']: invalid choice: 'foo'
 - literal['two']: invalid choice: 'foo'

which is still clear, but not as nice and concise. I think given that unions of only literals (which was the condition for this to work can and probably should be trivially converted to just a literal, this seems like a worthwhile tradeoff.

@DanCardin DanCardin force-pushed the dc/parse-capabale-choices branch from c71c3aa to 086397a Compare November 13, 2024 15:38
Copy link

codecov bot commented Nov 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (0c1b038) to head (8189a30).
Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #179   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           25        25           
  Lines         2294      2308   +14     
  Branches       499       500    +1     
=========================================
+ Hits          2294      2308   +14     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

@DanCardin DanCardin force-pushed the dc/parse-capabale-choices branch 2 times, most recently from 9a30e5d to 3c36afb Compare November 13, 2024 17:12
@DanCardin DanCardin force-pushed the dc/parse-capabale-choices branch from 3c36afb to 8189a30 Compare November 20, 2024 00:32
@DanCardin DanCardin marked this pull request as ready for review November 20, 2024 00:36
@DanCardin DanCardin merged commit 23a0be3 into main Nov 20, 2024
10 checks passed
@DanCardin DanCardin deleted the dc/parse-capabale-choices branch November 20, 2024 00:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Evaluate choices compliance after mapping, rather than in the parser
1 participant