-
Notifications
You must be signed in to change notification settings - Fork 292
feat: add 'millisecond' option to ser_json_timedelta config parameter #1427
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
Changes from 2 commits
0107915
21be1a2
537a484
f7a4008
f5df4d4
367da21
4af4625
c959789
5d80b34
c38fce0
abe32e6
9241bd7
61971a0
9aea492
6d54bd8
bff8e3b
00e68b7
b4c86de
574a011
a3900f6
fb80b83
d313c90
3e23511
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,7 +4,7 @@ use std::str::{from_utf8, FromStr, Utf8Error}; | |
use base64::Engine; | ||
use pyo3::intern; | ||
use pyo3::prelude::*; | ||
use pyo3::types::{PyDelta, PyDict, PyString}; | ||
use pyo3::types::{PyDelta, PyDict, PyFloat, PyString}; | ||
|
||
use serde::ser::Error; | ||
|
||
|
@@ -89,6 +89,7 @@ serialization_mode! { | |
"ser_json_timedelta", | ||
Iso8601 => "iso8601", | ||
Float => "float", | ||
Millisecond => "millisecond" | ||
} | ||
|
||
serialization_mode! { | ||
|
@@ -125,6 +126,14 @@ impl TimedeltaMode { | |
let seconds = Self::total_seconds(&py_timedelta)?; | ||
Ok(seconds.into_py(py)) | ||
} | ||
Self::Millisecond => { | ||
// convert to int via a py timedelta not duration since we know this this case the input would have | ||
// been a py timedelta | ||
let py_timedelta = either_delta.try_into_py(py)?; | ||
let seconds: f64 = Self::total_seconds(&py_timedelta)?.extract()?; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There might be a better way to do this which is alluding me, maybe we could do the multiplication in python? 🤷🏻♂️ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems reasonable enough - multiplication here should be faster. Can we pull out some of the shared logic into a function (both for Float and for Millisecond) and repeat that across the various branches? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hm, maybe, a couple of them use logic like:
However then the serializer needs to wrap these in Maybe a rust wizard could help with some clever refactoring im not seeing 🧙🏻 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To do the multiplication in Python would be something like let object = Self::total_seconds(&py_timedelta)?.mul(1000)?; ... this requires creating a Python integer But there is another inefficiency here (and in the other cases) which is that the call to The best option would be to go further and to add a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hmm, yeah neat suggestion on the I guess we'd want something like this: impl<'a> EitherTimedelta<'a> {
....
pub fn total_seconds(&self, py: Python<'a>) -> f64 {
match self {
Self::Raw(timedelta) => ...
Self::PyExact(py_timedelta) => ...
Self::PySubclass(py_timedelta) => ...
}
}
} And then we have two cases: Probably missed something here! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. pub fn total_seconds(&self) -> f64 {
match self {
Self::Raw(timedelta) => ...,
Self::PyExact(py_timedelta) => intern!(py_timedelta.py(), "total_seconds"))?.extract?
Self::PySubclass(py_timedelta) => intern!(py_timedelta.py(), "total_seconds"))?.extract?
}
} Something like this? Not 100% sure what to do with the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suggest looking at the For |
||
let object: Bound<PyFloat> = PyFloat::new_bound(py, seconds * 1000.0); | ||
Ok(object.into_py(py)) | ||
} | ||
} | ||
} | ||
|
||
|
@@ -139,6 +148,12 @@ impl TimedeltaMode { | |
let seconds: f64 = Self::total_seconds(&py_timedelta)?.extract()?; | ||
Ok(seconds.to_string().into()) | ||
} | ||
Self::Millisecond => { | ||
let py_timedelta = either_delta.try_into_py(py)?; | ||
let seconds: f64 = Self::total_seconds(&py_timedelta)?.extract()?; | ||
let milliseconds: f64 = seconds * 1000.0; | ||
Ok(milliseconds.to_string().into()) | ||
} | ||
} | ||
} | ||
|
||
|
@@ -159,6 +174,13 @@ impl TimedeltaMode { | |
let seconds: f64 = seconds.extract().map_err(py_err_se_err)?; | ||
serializer.serialize_f64(seconds) | ||
} | ||
Self::Millisecond => { | ||
let py_timedelta = either_delta.try_into_py(py).map_err(py_err_se_err)?; | ||
let seconds = Self::total_seconds(&py_timedelta).map_err(py_err_se_err)?; | ||
let seconds: f64 = seconds.extract().map_err(py_err_se_err)?; | ||
let milliseconds: f64 = seconds * 1000.0; | ||
serializer.serialize_f64(milliseconds) | ||
} | ||
} | ||
} | ||
} | ||
|
Uh oh!
There was an error while loading. Please reload this page.