Consistent validation with complex number to float type coercion #1574
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
CodSpeed Performance ReportMerging #1574 will not alter performanceComparing Summary
|
|
Hi @davidhewitt @sydney-runkle The CI is failing due to a pyright version mismatch (trying to install v1.1.389). I've verified that this version exists in the npm registry but for some reason CI was unable to fetch it. This seems to be an infrastructure issue rather than a problem with my changes. Would it be possible to get some guidance on this or have the CI re-run? And it would be great if my changes can be reviewed. Thank you for your time! |
| if strict { | ||
| return Err(ValError::new(ErrorTypeDefaults::FloatType, self)); | ||
| } | ||
| let real = self.getattr(intern!(self.py(), "real")).unwrap(); |
There was a problem hiding this comment.
Perhaps we should raise a warning here. In numpy's case, they do raise a custom ComplexWarning for discarding the imaginary part. At least we should document whatever coercion is implemented here because there can be a few possibilities for complex -> float:
- no coercion is allowed: consistent with the default python behaviour, e.g.
float(complex(1, 2))gives an error, even when the imaginary part is 0 - coercion is allowed only when the imaginary part is 0: this is what I would actually expect when coercions can actually take place
- coercion is allowed for all complex numbers, done by simply dropping the imaginary part: this seems to be a numpy-only behaviour and I'm not sure if non-numpy users would expect this
...if we really want to implement this coercion. Please take a look at my comment. pydantic/pydantic#10430 (comment)
There was a problem hiding this comment.
I am somewhat open to lax mode allowing complex -> float if the imaginary component is zero, similarly we allow datetime -> date if the datetime is at midnight.
I think we should never drop a nonzero imaginary part.
|
It looks like this got stuck for the last ~9 months following from #1574 (comment). Thank you for the PR, I will close it. If someone still wants the lax coercion when the imaginary part is zero then we can reopen as a new feature. |
Change Summary
When validating floats in non-strict mode, this PR adds support for properly handling complex to float coercion by
by extracting their real component, this behavior is prohibited in strict mode and returns ValidationError. Appropriate unit tests are added to
test_floatandtest_float_strict.Related issue number
Fixes pydantic/pydantic#10430
Checklist
pydantic-core(except for expected changes)Selected Reviewer: @davidhewitt