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

Fix datetime regression #743

Merged
merged 2 commits into from
Mar 22, 2013
Merged

Fix datetime regression #743

merged 2 commits into from
Mar 22, 2013

Conversation

tomchristie
Copy link
Member

Refs #740.

@toranb
Copy link
Contributor

toranb commented Mar 21, 2013

Looks good -thanks for taking the time to do such a thorough pull request Tom!

@benrobster
Copy link
Contributor

Looks good AFAICT. Thanks for addressing this! I was hoping to get around to figuring out a fix but you beat me to it and did a better job than I could have.

@minddust
Copy link
Contributor

i'm not sure if this behavior is right. shouldn't to_native return a string and not an object?

@benrobster
Copy link
Contributor

I guess its a design decision to be made. I wouldn't say that to_native
returning a date or datetime is objectively wrong, since date & datetime
are certainly core native python types and serialization in DRF up to this
point has been about converting to native python, whereas rendering has
been more about converting to strings. When we do to_native on an int,
float, Decimal, list, tuple, or dictionary, we don't return a string
representation of a int, float, Decimal, list, tuple, or dictionary, but
the native type itself. Datetime may be be a bit more of a complex
"native" type, but its still a core python native. On the other hand, there
are certainly other object types that are built-in to python that don't
fall within the "protected types" defined by DRF and would be converted to
strings during serialization. I obviously am biased in this situation
since I currently have a reliance on it being a datetime that I would need
to address, but I think what's best for the framework is more important
than not breaking my specific case (and i already have a simple workaround
override, so its not a huge deal). So if having the output be a string (the
functionality in the current release) is the most consistent thing to do
here, lets do that. (See my other reply on #740)

On Fri, Mar 22, 2013 at 2:19 AM, Stephan Groß [email protected]:

i'm not sure if this behavior is right. shouldn't to_native return a
string and not an object?


Reply to this email directly or view it on GitHubhttps://github.com//pull/743#issuecomment-15285575
.

@tomchristie
Copy link
Member Author

Having datetime objects returned from the fields and treated as one of the 'primitive' types seems reasonable to me, and there's certainly a case to be made that how those should be handled is at least partly a renderer concern.

For example the current JSONRenderer uses ECMA 262 (the javascript spec) compliant date time strings. (Subset of iso8601) But other renderer types might have other requirements about how datetimes should be represented.

Anyways short story is that on reflection I'm happy this is the right thing to do.

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.

4 participants