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

Add enum error type #751

Merged
merged 2 commits into from
Jul 10, 2023
Merged

Add enum error type #751

merged 2 commits into from
Jul 10, 2023

Conversation

hramezani
Copy link
Member

@hramezani hramezani commented Jul 7, 2023

Related to pydantic/pydantic#6439

The documentation PR in pydantic needs this error type to be added to pydantic-core

Selected Reviewer: @dmontagu

@codecov
Copy link

codecov bot commented Jul 7, 2023

Codecov Report

Merging #751 (1ef03d6) into main (fac2a40) will decrease coverage by 0.01%.
The diff coverage is 75.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #751      +/-   ##
==========================================
- Coverage   93.64%   93.64%   -0.01%     
==========================================
  Files          99       99              
  Lines       13929    13933       +4     
  Branches       25       25              
==========================================
+ Hits        13044    13047       +3     
- Misses        879      880       +1     
  Partials        6        6              
Impacted Files Coverage Δ
python/pydantic_core/core_schema.py 96.64% <ø> (ø)
src/errors/types.rs 83.96% <75.00%> (-0.10%) ⬇️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fac2a40...1ef03d6. Read the comment docs.

@codspeed-hq
Copy link

codspeed-hq bot commented Jul 7, 2023

CodSpeed Performance Report

Merging #751 will not alter performances

Comparing enum_error_type (ec6be31) with main (fac2a40)

Summary

✅ 126 untouched benchmarks

@hramezani
Copy link
Member Author

please review

Copy link

@alfawal alfawal left a comment

Choose a reason for hiding this comment

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

Don't we need to include it at tests/serializers/test_any.py:381 (The test_num function)?

@@ -159,6 +159,11 @@ pub enum ErrorType {
pattern: String,
},
// ---------------------
// enum errors
Enum {
options: String,
Copy link
Contributor

Choose a reason for hiding this comment

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

Once merged, I suppose this will be used to replace https://github.com/pydantic/pydantic/blob/b685d64c38f1d7abb3ed37bd30710b3e420dc86a/pydantic/_internal/_std_types_schema.py#L83 with a PydanticKnownError?

Is there a reason why you chose options here instead of expected from that line? I see expected is also used in other places in this file.

Copy link
Member Author

Choose a reason for hiding this comment

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

Didn't know about this.

Thanks for mentioning this. I've changed them to expected

@hramezani
Copy link
Member Author

Don't we need to include it at tests/serializers/test_any.py:381 (The test_num function)?

No, I think this is related to validation

@davidhewitt davidhewitt merged commit a27b34e into main Jul 10, 2023
28 checks passed
@davidhewitt davidhewitt deleted the enum_error_type branch July 10, 2023 14:28
adriangb pushed a commit that referenced this pull request Jul 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants