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

Parse datetime timezone #251

Closed
wants to merge 1 commit into from

Conversation

luiscoms
Copy link

@luiscoms luiscoms commented Feb 11, 2019

Add timezone when parse datetime

When I load an yaml like yaml.load("2018-02-11 19:00:00-02:00")

I got

2018-02-11 21:00:00

Instead of

2018-02-11 19:00:00-02:00

@luiscoms luiscoms force-pushed the constructor-datetime branch from 4e96ac8 to 8d7aaf1 Compare February 11, 2019 21:01
@luiscoms luiscoms changed the title WIP: Return datetime timezoned Parse datetime timezone Feb 11, 2019
@luiscoms
Copy link
Author

Any reviews on this PR?

@ingydotnet
Copy link
Member

@luiscoms After the 5.1 release, I expect to review this PR (and many others) for the next release.

Thank you for the work.

@seansfkelley
Copy link

seansfkelley commented Apr 20, 2019

See also #163. I prefer the code style in this PR, but #163 uses standard library timezone objects when available, which seems preferable.

@luiscoms
Copy link
Author

Here is user datetime.timezone to parsing timezone https://github.com/yaml/pyyaml/pull/251/files#diff-a5f8b847ff620043866fa0e0d854af0dR334

@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

See also #163. I prefer the code style in this PR, but #163 uses standard library timezone objects when available, which seems preferable.

Could you explain the difference to #163? I'm still new to python.

@seansfkelley
Copy link

https://github.com/yaml/pyyaml/pull/163/files#diff-cf4d4f120c4c496f37b4d3d84f585b46R357-R363 is not very DRY.

https://github.com/yaml/pyyaml/pull/251/files#diff-cf4d4f120c4c496f37b4d3d84f585b46R348 seems preferable.

The two PRs seem to have converged a bit in order to support Python 2. When I said it uses "standard library time zones", I meant that it didn't implement its own class, which it now does as of 12d5efd. Also, the implementations of timezone are currently different between these two PRs. It's been a while since I've looked at time zone stuff in Python, but not wrapping standard library things seems better from a compatibility perspective too (I've had issues mixing zone objects from different libraries that should have been interchangeable, but were not).

@perlpunk
Copy link
Member

perlpunk commented Dec 9, 2019

Thanks. I would like to get this in the next release, but won't have much time until next weekend.

@perlpunk
Copy link
Member

I merged #163
I also created #363 to add timezone tests. Maybe you could have a look if it makes sense?

@perlpunk
Copy link
Member

perlpunk commented Jan 6, 2020

released https://pypi.org/project/PyYAML/5.3/

@perlpunk perlpunk closed this Jan 6, 2020
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.

5 participants