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

Support serialization as float in TimeDelta field #1998

Merged
merged 4 commits into from
Jun 26, 2022

Conversation

marcosatti
Copy link
Contributor

Add functionality to (de)serialize to float values as a representation of the total seconds.

In our use case, we are trying to specify a period which is a combination of seconds, milliseconds and microseconds, which is easier to represent as a single floating point value rather than specifying the smallest unit (microseconds). We have currently been serializing to a fields.Float() using .total_seconds() and the equivalent to deserialize.

The feature does not promise absolute precision, which is stated in the docs (same restrictions as floating point values).

This does not change the current way TimeDelta is used.

Able to (de)serialise to float values as a representation of the total seconds.
Copy link
Member

@lafrech lafrech left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution.

I understand your use case.

I'm reluctant to integrate this as is, though. At this point, we might as well offer int and float for any precision. This would have the benefit of symmetry, the possibilities would be the cartesian product of {all precisions} x {int, float}. And one could serialize to a floating point number of days.

You'd just have to add a new parameter expressing int or float (int by default).

Just my thoughts, I could be convinced otherwise and maybe other maintainers will chime in with a different view.

tests/test_serialization.py Outdated Show resolved Hide resolved
@marcosatti
Copy link
Contributor Author

Having a serde_type parameter does sound like a more complete approach to the problem - I'd be ok to add that in. I'll wait for more discussion first.

@marcosatti
Copy link
Contributor Author

Went ahead and updated anyway to support all precisions through use of the serde_type parameter.

Copy link
Member

@lafrech lafrech left a comment

Choose a reason for hiding this comment

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

Nice, thanks!

Just minor comments.

src/marshmallow/fields.py Outdated Show resolved Hide resolved
src/marshmallow/fields.py Outdated Show resolved Hide resolved
src/marshmallow/fields.py Outdated Show resolved Hide resolved
- Implement a test for exception being raised on invalid type parameter.
- Rename serde_type to serialization_type.
- Condense code.
Copy link
Member

@lafrech lafrech left a comment

Choose a reason for hiding this comment

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

LGTM. Just a few comments.

I'll wait a few days in case other maintainers want to review as well.

src/marshmallow/fields.py Show resolved Hide resolved
src/marshmallow/fields.py Outdated Show resolved Hide resolved
@marcosatti
Copy link
Contributor Author

Fixed up remaining documentation issues.

@lafrech lafrech changed the title Add total seconds float support to the TimeDelta field Support serialization as float in TimeDelta field Jun 26, 2022
@lafrech lafrech merged commit 950d972 into marshmallow-code:dev Jun 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants