-
-
Notifications
You must be signed in to change notification settings - Fork 629
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
fields.DateTime - load/dump to timestamp #612
Comments
@vgavro I think this is a fair use case. I anticipate that users will typically want to deserialize to timezone-aware datetimes, so the API should make this simple. I haven't given to much thought to what it would look like though. Maybe it would make sense as a separate field that receives a my_timestamp = fields.Timestamp(tzinfo=pytz.utc) |
Bump, this would be nice to have 👍 |
Is there a plan to do this thing? I can't tell you how happy this would make me? |
@stephan-rayner You can do it even better: work on that and send a PR? :) Note that you can also attend in https://hacktoberfest.digitalocean.com and win a t-shirt ;D. |
I would gladly review and merge a PR adding a |
I will gladly work on this PR, but maybe it's better to implement this as custom format for AwareDateTime and NaiveDateTime instead of separate field #933 ? Like I see it's good decision to force conversion to aware datetime, but still explicitly allow naive datetimes. |
explicitly is the key here. If the user wants it, let him do that. I am not a maintainer of the project but I am up for this as we wouldn't make the bad thing (naive datetimes) to be the default ones. |
Ah, yes, I see your @vgavro . Yes, I suppose it could be a new datetime format. I'm fine with that. |
@sloria I've tried this as a format for
My idea is that supporting timestamps is a bit more than formatting datetime to float. |
@lig As you can see in implementation of @sloria @lafrech Here are scenarios with timestamps (+ timezone aware/naive datetimes) we want to cover, I would really love to have feedback before I can work on this, if I haven't anything missed (I hope?) [1]. By definition timestamp (POSIX time or UNIX Epoch time) should be in UTC. So generally timestamp adding because of [1] can be solved: As for [2],[3] - one option may be to interpret timestamp timezones (or other datetimes) in custom ways (replace/drop/convert tzinfo) in subclasses, that willl inherit DateTime, but not required to be in it's core functionality. This way problem is partly trying to solve #933 , but in case with timestamp we want to explicitly replace timezone, not just default it (because timestamp default is UTC, like we see in [1]). I think this functionality can be in base The [2] problem may be solved: The [3] problem may be solved: Cons on this:
P.S. One more use-case that personally I see not fits in conception "timestamp is just another datetime format" is interpreting 0 as null for timestamps, but it's too easy to implement in user space and really is just bad api design that I've seen - but I'm sure we don't need it in marshmallow code anyway. I really think this will fix timezones/timestamps issues once for all (at last), sorry for tagging (I know it's hard to give time for such big open source project), and thanks in advance for your feedback. I can use tests and |
@vgavro my concern regarding the type of serialization result For example, OpenAPI plugin produces openapi spec |
timestamps as
pros:
I think it's very important to have consistent behaviour about timezone-aware/naive things between datetime fields, and I don't see how separate |
Quick answer @vgavro as I don't have time to look at it right now.
|
@vgavro another case to mention is about possible value ranges. The thing is DateTime could be whatever in the past. However, only values from 1970-01-01 are possible to serialize as a timestamp. |
@lig that affects nothing, because timestamps can be negative.
|
@lafrech I just wanted to solve this aware/naive functionality in place, because default timezone and explicitly converted naive datetime is really widely-used case. My first proposal exactly was rethinking your PR also, but like I see there is a lot of discussion needed and it's better be solved in separate (with timestamp) ways, I really concerned about timezone-aware/naive DateTime field, but like I see now it's better not to mix this decisions. |
I didn't go through the whole thread again but I'm pretty sure this feature has become much easier to implement thanks to the rework in #1278. |
any updates on fields.Timestamp? |
|
Bump, would love to have this |
It's a very goog idea. It's a very common case, the datetime object from sqlalchemy is without timezone. At most cases, the right tz is system tz. But maybe reanme to LocalDatetTime? |
I'm currently using this code to parse/dump timestamp, but I believe this should be in standard library. Should I create pull request, or have I missed another way to work with datetime/timestamp format?
The text was updated successfully, but these errors were encountered: