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

On load, create timezone-aware datetime objects if possible #113

Closed
wants to merge 1 commit into from

Conversation

cdavoren
Copy link

@cdavoren cdavoren commented Dec 18, 2017

On loading data, if timestamps have an ISO "+HH:MM" UTC offset then the resultant datetime is currently converted to UTC. This change creates a timezone-aware datetime when this happens. If there is no offset, a naive datetime is created as per the previous version.

Importantly, this addresses a Django warning (and potential error) that appears when using YAML fixtures in a timezone-aware project. It was raised as a Django issue (https://code.djangoproject.com/ticket/18867), but subsequently closed because the Django devs felt that this is a PyYAML problem.

On loading data, if timestamps have an ISO "+HH:MM" UTC offset then the resultant datetime is converted to UTC.  This change adds that timezone information to the datetime objects.

Importantly, this addresses a Django warning (and potential error) that appears when using both YAML fixtures in a timezone-aware project.  It was raised as a Django issue (https://code.djangoproject.com/ticket/18867), but subsequently closed because the Django devs felt that this is a PyYAML problem.
@cdavoren
Copy link
Author

Build fails on python 2.6 due to dropped support by Python team (I think? Not sure how to interpret the logs exactly). I don't think this is an error in the pull request.

Copy link

@grabekm90 grabekm90 left a comment

Choose a reason for hiding this comment

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

Nice work!
I have tested the patch using my fixtures and now the warning does not appear.

@cdavoren
Copy link
Author

Thanks, glad to help!

@cnk
Copy link

cnk commented Apr 14, 2018

Hey any movement on this? From what I can see in the CI output, I don't think python 2.6 is ever going to pass - and all the other tests pass.

@perlpunk
Copy link
Member

perlpunk commented Apr 14, 2018

I think this makes sense.
Since a YAML timestamp without timezone information is considered UTC, maybe the timezone should be set in every case.

@akaIDIOT
Copy link
Contributor

akaIDIOT commented May 7, 2018

Looking at the implementation, the timezone is always set to UTC, with the actual datetime being shifted. That seems wrong; if we're going to parse the datetime, we ought to take the values as intended.

@mattseymour
Copy link

Is there an ETA on supporting TZ date times. Currently 163, 113 and 251 are all open which try to resolve this issue. It seems to be an issue being faced by a lot of people and the fact 3 pull requests are open for such an issue highlights this.

It would be good if someone from the core team could shed some light on why these changes have taken a while to get reviewed so we may assist where possible; whether it is that the devs are busy or have reservations about the PR's. In either case, it would be good to start a conversation around this so we can all contribute to help push this change into master (sooner rather than later).

Cross-posting to other tickets.

@perlpunk
Copy link
Member

perlpunk commented Dec 9, 2019

I think I'll close this one as it only changes lib3. #163 seems like the one we should merge.
I'll try to get this into v5.3 or v5.4

@perlpunk perlpunk closed this Dec 9, 2019
@perlpunk perlpunk removed this from Backlog in 5.3 Release Dec 9, 2019
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.

None yet

6 participants