Skip to content

JSONRender fails to serialize timedelta objects #392

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

Closed
radiosilence opened this issue Nov 9, 2012 · 10 comments
Closed

JSONRender fails to serialize timedelta objects #392

radiosilence opened this issue Nov 9, 2012 · 10 comments

Comments

@radiosilence
Copy link
Contributor

It's also not immediately clear as to how to override the behaviour.

@tomchristie
Copy link
Member

See the encoder implementation: https://github.com/tomchristie/django-rest-framework/blob/master/rest_framework/utils/encoders.py#L13

(Not documented as it's considered a private api)

@asfaltboy
Copy link
Contributor

For timedelta, we can take a look at Mark Hildreth's approach:
http://taketwoprogramming.blogspot.co.il/2009/06/subclassing-jsonencoder-and-jsondecoder.html

more precisely:

elif isinstance(obj, timedelta):
            days = obj.days
            seconds = obj.seconds
            milliseconds = obj.microseconds / 1000
            milliseconds += obj.seconds * 1000
            milliseconds += obj.days * 24 * 60 * 60 * 1000

            return 'td(%d)' % (milliseconds)

@tomchristie
Copy link
Member

I don't think that approach is quite appropriate here since it encodes into such a specific format.
I think we should simply encode timedeltas into '1234.567' strings (Note Decimals also necessarily get string representations).

NB. In the other direction there's no need for the JSON decoder to be able to determine and decode the type into a timedelta automagically, as that's a serializer concern.

@radiosilence
Copy link
Contributor Author

I think strings would be fine, because the serializer *knows *what we're
going to be decoding to, and a timedelta is just a bunch of settings.
Should I figure out my own subclassed decoder or do you think this will be
in upstream?

James Cleveland
Web Developer
telephone? +44(0)7865 99 88 57
website? www.dapperdogstudios.com

On 9 November 2012 15:55, Tom Christie [email protected] wrote:

I don't think that approach is quite appropriate here since it encodes
into such a specific format.
I think we should simply encode timedeltas into '1234.567' strings (Note
Decimals also necessarily get string representations).

NB. In the other direction there's no need for the JSON decoder to be able
to determine and decode the type into a timedelta automagically, as that's
a serializer concern.


Reply to this email directly or view it on GitHubhttps://github.com//issues/392#issuecomment-10233971.

@radiosilence
Copy link
Contributor Author

There's also the option of doing str(mytimedelta) which works, and to
decode you could do timedelta(*mystr.split(':')) (except not quite)

James Cleveland
Web Developer
telephone? +44(0)7865 99 88 57
website? www.dapperdogstudios.com

On 9 November 2012 15:57, James Cleveland [email protected] wrote:

I think strings would be fine, because the serializer *knows *what we're
going to be decoding to, and a timedelta is just a bunch of settings.
Should I figure out my own subclassed decoder or do you think this will be
in upstream?

James Cleveland
Web Developer
telephone? +44(0)7865 99 88 57
website? www.dapperdogstudios.com

On 9 November 2012 15:55, Tom Christie [email protected] wrote:

I don't think that approach is quite appropriate here since it encodes
into such a specific format.
I think we should simply encode timedeltas into '1234.567' strings (Note
Decimals also necessarily get string representations).

NB. In the other direction there's no need for the JSON decoder to be
able to determine and decode the type into a timedelta automagically, as
that's a serializer concern.


Reply to this email directly or view it on GitHubhttps://github.com//issues/392#issuecomment-10233971.

@radiosilence
Copy link
Contributor Author

So yeah, seconds is probably best, because I just realised that str(delta)
comes up with shit like "4 days, 12:23:32" which would be a pain to decode.

James Cleveland
Web Developer
telephone? +44(0)7865 99 88 57
website? www.dapperdogstudios.com

On 9 November 2012 15:58, James Cleveland [email protected] wrote:

There's also the option of doing str(mytimedelta) which works, and to
decode you could do timedelta(*mystr.split(':')) (except not quite)

James Cleveland
Web Developer
telephone? +44(0)7865 99 88 57
website? www.dapperdogstudios.com

On 9 November 2012 15:57, James Cleveland [email protected]:

I think strings would be fine, because the serializer *knows *what we're
going to be decoding to, and a timedelta is just a bunch of settings.
Should I figure out my own subclassed decoder or do you think this will be
in upstream?

James Cleveland
Web Developer
telephone? +44(0)7865 99 88 57
website? www.dapperdogstudios.com

On 9 November 2012 15:55, Tom Christie [email protected] wrote:

I don't think that approach is quite appropriate here since it encodes
into such a specific format.
I think we should simply encode timedeltas into '1234.567' strings (Note
Decimals also necessarily get string representations).

NB. In the other direction there's no need for the JSON decoder to be
able to determine and decode the type into a timedelta automagically, as
that's a serializer concern.


Reply to this email directly or view it on GitHubhttps://github.com//issues/392#issuecomment-10233971.

@tomchristie
Copy link
Member

Should I figure out my own subclassed decoder or do you think this will be
in upstream?

Into core is good.

@tomchristie
Copy link
Member

@radiosilence

Yo. Made something for ya. ⚡ 😏 ⚡

pullrequest

@radiosilence
Copy link
Contributor Author

#584 :O

@minddust
Copy link
Contributor

yeah! :D
nice meme 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants