-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
Support complex number #9654
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
Support complex number #9654
Conversation
|
Hi @changhc, Great start here - looking good. Maybe we could add a section to the docs as well with some usage patterns. |
|
I left some notes on next steps in the |
CodSpeed Performance ReportMerging #9654 will not alter performanceComparing Summary
|
|
@changhc, could you provide an example in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, pending revision of the complex_str_parsing error docs in another PR!
|
Probably could use some more docs here as well. I can handle that and will create an associated issue. |
|
Hi @sydney-runkle, I've updated the error doc. The test case that failed with pypy is now skipped. A fix is merged into pypy master branch but not yet released. I'm trying to fix the failed mypy test. |
| @@ -1,56 +1,56 @@ | |||
| from pydantic import BaseModel, Field | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure what is actually changed in this file. Everything looks identical. It was "updated" after I ran make test-mypy-update-all.
Coverage reportClick to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||||||||||||||||||||
|
Awesome find! Great work on this feature. We'll be sure to celebrate this in our upcoming release!! I recall you mentioned you might be interested in working on some other issues. If this is still the case, I can point you towards some fun ones! |
|
Thank you for the review! Yes I would love to look into other issues. By the way, how much does the team care about test coverage? After reading the test coverage report, I'm now adding some tests to cover the missing lines, though I'm not sure if they actually help since the tests to be added are like simply checking if the json schema of a model matches the expected schema. |
We care a decent amount about coverage - more tests sound great. Happy to merge after we add those. We generally try to keep coverage above 90%, but closer to 100% would be great.
Awesome, I'll ping you with some fun ones soon. |
|
The missing line in the previous report is now covered. |
Change Summary
Support complex numbers.
Related issue number
Closes #8555
Checklist
Also, upgrade to
pydantic-corev2.22.0, which adds support for complex schemas. We've bundled in some other patches along with this release, such as removing xfails associated with tests that this release fixed!