bpo-31800: Support for colon when parsing time offsets #4015

mariocj89 · 2017-10-16T22:54:17Z

Add support to strptime to parse time offsets with a colon between the hour and the minutes.

https://bugs.python.org/issue31800

mariocj89 · 2017-10-21T13:04:16Z

included the support for 'Z' in 49098f6 happy to remove it or extract it in a different PR.

abalkin · 2017-10-23T19:30:53Z

Doc/library/datetime.rst

+      When ``%z`` directive is provided to the :meth:`strptime` method, any valid
+      RFC-822/ISO 8601 standard utftime-offset can be parsed. For example, strings
+      like ``'+01:00'`` will be parsed as an offset of one hour and or ``'Z'`` will
+      be transofmred to a 0 utc offset.


Please use all caps for "UTC".

abalkin · 2017-10-23T19:35:56Z

Lib/_strptime.py

@@ -210,7 +210,7 @@ def __init__(self, locale_time=None):
            #XXX: Does 'Y' need to worry about having less or more than
            #     4 digits?
            'Y': r"(?P<Y>\d\d\d\d)",
-            'z': r"(?P<z>[+-]\d\d[0-5]\d)",
+            'z': r"(?P<z>[+-]\d\d:?[0-5]\d|Z)",


Please add support for sub-minute offsets. See bpo-5288.

bedevere-bot · 2017-10-23T19:36:08Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

This change adds support to _strptime to parse time offsets with a colon between the hour and the minutes. This unblocks the parsing of datetime string representation generated via isoformat together with all string that have the colon in its offset (which is quite common and part of the ISO8606)

mariocj89 · 2017-10-23T23:20:09Z

I have made the requested changes; please review again

bedevere-bot · 2017-10-23T23:20:11Z

Thanks for making the requested changes!

@abalkin: please review the changes made to this pull request.

mariocj89 · 2017-10-23T23:21:16Z

Added both support for second and microseconds worries me that now the offset can be both a float and an int depending on whether there are microseconds in it though.

Open to suggestions!

Thanks

abalkin · 2017-10-24T00:07:50Z

@mariocj89 - the offset is a timedelta, not int or float. Let me take a look at your changes ...

abalkin · 2017-10-24T00:15:34Z

Lib/_strptime.py

+                seconds = int(z[5:7] or 0)
+                microseconds = int(z[8:] or 0)
+                gmtoff = (hours * 60 * 60) + (minutes * 60) + seconds
+                gmtoff = gmtoff + (microseconds / (10**6))


Let's not make gmtoff a float, but instead add gmtoff_fraction to the returned tuple. See how %f format is handled.

mariocj89 · 2017-10-24T09:11:39Z

Let's not make gmtoff a float, but instead add gmtoff_fraction to the returned tuple. See how %f format is handled.

Great idea, I've added it at the end like the fraction, if you rather want it as part of the time tuple we can try that out as well.

Thanks @abalkin !

mariocj89 · 2017-10-25T08:22:52Z

@abalkin I've added some further information & corrected a typo in the NEWS entry.

Do you want me to add some info in Python3.7 what's new as well or it is not really worth it?

abalkin · 2017-10-25T16:42:43Z

Doc/library/datetime.rst

@@ -2174,6 +2174,12 @@ Notes:
   .. versionchanged:: 3.7
      The UTC offset is not restricted to a whole number of minutes.

+   .. versionchanged:: 3.7
+      When ``%z`` directive is provided to the :meth:`strptime` method, any valid
+      RFC-822/ISO 8601 standard utftime-offset can be parsed. For example, strings


utftime-offset? This should be UTC offset, right?

abalkin · 2017-10-25T16:54:24Z

Misc/NEWS.d/next/Library/2017-10-17-20-08-19.bpo-31800.foOSCi.rst

@@ -0,0 +1,3 @@
+Add support for parsing RFC-822/ISO 8601 UTC offsets. strptime '%z' can now


I don't think the RFC or the ISO standard allows seconds, let alone microseconds in the UTC offset. Please note that RFC 822 was obsoleted by RFC 2822 which in turn was obsoleted by 5322. I don't think this matters much for this PR because the RFCs specify the TZ format without a colon. I would remove references to standards and just describe what is implemented.

bedevere-bot · 2017-10-25T16:54:33Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

mariocj89 · 2017-10-25T17:56:40Z

I have made the requested changes; please review again

bedevere-bot · 2017-10-25T17:56:42Z

Thanks for making the requested changes!

@abalkin: please review the changes made to this pull request.

abalkin · 2017-10-26T00:34:04Z

Do you want me to add some info in Python3.7 what's new as well or it is not really worth it?

A NEWS entry is enough. The What's New editor will pick it from there.

the-knights-who-say-ni added the CLA signed label Oct 16, 2017

bedevere-bot added the awaiting review label Oct 16, 2017

mariocj89 force-pushed the offset_colon branch from d13d073 to bf46b6a Compare October 17, 2017 23:08

abalkin self-assigned this Oct 19, 2017

abalkin requested a review from vstinner October 21, 2017 20:14

vstinner requested a review from abalkin October 23, 2017 17:22

abalkin requested changes Oct 23, 2017

View reviewed changes

bedevere-bot added awaiting changes and removed awaiting review labels Oct 23, 2017

mariocj89 force-pushed the offset_colon branch 2 times, most recently from 16227fa to 59af357 Compare October 23, 2017 22:38

mariocj89 and others added 10 commits October 23, 2017 23:57

Add news entry

0facd08

Docs for parsing using %z and colons

af68297

Add support for Z on %z directive

e0e3873

Update docs to reflect the addition of parsing Z

7c53307

Add support for parsing seconds

d184e7b

Add support for microsecond on strptime

3809303

Add test case with the new format on datetimetester

7548704

Improve error message on : invalid use

69e655c

Typo: utc -> UTC

f718b37

mariocj89 force-pushed the offset_colon branch from 59af357 to f718b37 Compare October 23, 2017 22:59

bedevere-bot removed the awaiting changes label Oct 23, 2017

bedevere-bot added the awaiting change review label Oct 23, 2017

abalkin reviewed Oct 24, 2017

View reviewed changes

Handle gmtoff miliseconds via a separate variable

5cfeeee

abalkin approved these changes Oct 25, 2017

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting change review labels Oct 25, 2017

Enrich NEWS entry with latest changes

546b7b8

abalkin requested changes Oct 25, 2017

View reviewed changes

bedevere-bot added awaiting changes and removed awaiting merge labels Oct 25, 2017

Update docs on new %z behaviour

c20b57f

bedevere-bot added awaiting change review and removed awaiting changes labels Oct 25, 2017

abalkin approved these changes Oct 25, 2017

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting change review labels Oct 25, 2017

abalkin merged commit 3231893 into python:master Oct 26, 2017

bedevere-bot removed the awaiting merge label Oct 26, 2017

mariocj89 mentioned this pull request Dec 10, 2017

bpo-32267: Fix parsing tz offsets microseconds shorter than 6 #4781

Merged

mariocj89 deleted the offset_colon branch January 6, 2018 17:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bpo-31800: Support for colon when parsing time offsets #4015

bpo-31800: Support for colon when parsing time offsets #4015

mariocj89 commented Oct 16, 2017 •

edited by abalkin

Loading

mariocj89 commented Oct 21, 2017

abalkin Oct 23, 2017

abalkin Oct 23, 2017

bedevere-bot commented Oct 23, 2017

mariocj89 commented Oct 23, 2017

bedevere-bot commented Oct 23, 2017

mariocj89 commented Oct 23, 2017 •

edited

Loading

abalkin commented Oct 24, 2017

abalkin Oct 24, 2017

mariocj89 commented Oct 24, 2017

mariocj89 commented Oct 25, 2017

abalkin Oct 25, 2017

abalkin Oct 25, 2017

bedevere-bot commented Oct 25, 2017

mariocj89 commented Oct 25, 2017

bedevere-bot commented Oct 25, 2017

abalkin commented Oct 26, 2017

		@@ -0,0 +1,3 @@
		Add support for parsing RFC-822/ISO 8601 UTC offsets. strptime '%z' can now

bpo-31800: Support for colon when parsing time offsets #4015

bpo-31800: Support for colon when parsing time offsets #4015

Conversation

mariocj89 commented Oct 16, 2017 • edited by abalkin Loading

mariocj89 commented Oct 21, 2017

abalkin Oct 23, 2017

Choose a reason for hiding this comment

abalkin Oct 23, 2017

Choose a reason for hiding this comment

bedevere-bot commented Oct 23, 2017

mariocj89 commented Oct 23, 2017

bedevere-bot commented Oct 23, 2017

mariocj89 commented Oct 23, 2017 • edited Loading

abalkin commented Oct 24, 2017

abalkin Oct 24, 2017

Choose a reason for hiding this comment

mariocj89 commented Oct 24, 2017

mariocj89 commented Oct 25, 2017

abalkin Oct 25, 2017

Choose a reason for hiding this comment

abalkin Oct 25, 2017

Choose a reason for hiding this comment

bedevere-bot commented Oct 25, 2017

mariocj89 commented Oct 25, 2017

bedevere-bot commented Oct 25, 2017

abalkin commented Oct 26, 2017

mariocj89 commented Oct 16, 2017 •

edited by abalkin

Loading

mariocj89 commented Oct 23, 2017 •

edited

Loading