Fraction serialization in Python mode #1854
Conversation
There was a problem hiding this comment.
Thanks very much for this, overall looks great.
A bunch of comments. Also please be aware that we're considering moving pydantic-core into the pydantic tree (pydantic/pydantic#12436) to make this sort of development of large features easier to write, document and test in one shot.
Depending on the rate of progress you might need to import this PR onto the pydantic repo PR, probably with git subtree pull or similar, we can work that out together.
| type: Required[Literal['decimal']] | ||
| le: Decimal | ||
| ge: Decimal | ||
| lt: Decimal | ||
| gt: Decimal |
There was a problem hiding this comment.
Think some decimal -> fraction replacement needed here.
| // let mut is_nan: Option<bool> = None; | ||
| // let mut is_nan = || -> PyResult<bool> { | ||
| // match is_nan { | ||
| // Some(is_nan) => Ok(is_nan), | ||
| // None => Ok(*is_nan.insert(decimal.call_method0(intern!(py, "is_nan"))?.extract()?)), | ||
| // } | ||
| // }; |
There was a problem hiding this comment.
nan is presumably not relevant here.
| // } | ||
| // }; | ||
|
|
||
| // if let Some(le) = &self.le { |
There was a problem hiding this comment.
We should implement all these (and add tests for validation).
| ) -> PyResult<Py<PyAny>> { | ||
| let _py = value.py(); | ||
| match extra.ob_type_lookup.is_type(value, ObType::Fraction) { | ||
| IsType::Exact | IsType::Subclass => infer_to_python_known(ObType::Fraction, value, include, exclude, extra), |
There was a problem hiding this comment.
Unfortunately to avoid breaking changes in pydantic v2 we will need a config flag for the change of Python serialization. (Users can then opt-in to the fixed behaviour and config flag can be deprecated / removed in v3.)
There was a problem hiding this comment.
I would say a config flag is probably too much for this use case (no one reported this -- pydantic/pydantic#12286 was created by my when rewriting the types documentation), and users can define a custom serializer until v3.
| ) -> ValResult<EitherInt<'py>> { | ||
| let py = fraction.py(); | ||
|
|
||
| // as_integer_ratio was added in Python 3.8, so this should be fine |
There was a problem hiding this comment.
| // as_integer_ratio was added in Python 3.8, so this should be fine |
I don't think it's necessary to mention addition of such methods, as we don't support 3.8 anyway.
| ) -> PyResult<Py<PyAny>> { | ||
| let _py = value.py(); | ||
| match extra.ob_type_lookup.is_type(value, ObType::Fraction) { | ||
| IsType::Exact | IsType::Subclass => infer_to_python_known(ObType::Fraction, value, include, exclude, extra), |
There was a problem hiding this comment.
I would say a config flag is probably too much for this use case (no one reported this -- pydantic/pydantic#12286 was created by my when rewriting the types documentation), and users can define a custom serializer until v3.
Co-authored-by: David Hewitt <[email protected]>
3 participants
Change Summary
Add validation and serialization for
fractions.Fraction, heavily inspired by how this is done for Decimal.Related issue number
supports pydantic/pydantic#12286
This PR works in conjuction with:
pydantic/pydantic#12442
Checklist
[ ] Documentation reflects the changes where applicablepydantic-core(except for expected changes)Open todos:
le, ge, lt, gtmultiple_ofas forDecimal?