Closes issue bpo-5288: Allow tzinfo objects with sub-minute offsets.#2896
Closes issue bpo-5288: Allow tzinfo objects with sub-minute offsets.#2896abalkin merged 6 commits intopython:masterfrom
Conversation
|
The comments above the functions and the documentation still mention a 'whole number of minutes'. Do these need to be modified? |
|
@csabella - yes the comments and the documentation should be fixed. Can you point me to the specific instances? |
|
Thanks @abalkin. I'm not able to leave a comment in the code on lines that haven't been modified, but it's line 244 in datetime.py, right above the In the datetime.rst, it's probably easiest to search on the word |
|
@csabella - please see 97d8d5b. |
|
Thank you. Looks good. You even caught the ones in the C code that I had missed. Can you just recheck the |
vstinner
left a comment
There was a problem hiding this comment.
In general, LGTM, just minor nits.
But I have a question for %z: does it support microseconds or not? According to the doc, it doesn't: so please clarify the ".. versionchanged:: 3.7" note.
| seconds = rest.seconds | ||
| microseconds = rest.microseconds | ||
| if microseconds: | ||
| return 'UTC{}{:02d}:{:02d}:{:02d}.{:06d}'.format(sign, |
| ``'-0330'``. | ||
|
|
||
| .. versionchanged:: 3.7 | ||
| The UTC offset is not restricted to a whole number of minutes. |
There was a problem hiding this comment.
I suggest to be more explicit: mention that optional support for seconds was added, since microseconds are not supported here.
There was a problem hiding this comment.
Actually, microseconds are supported here. Will larigy.
| hours = divmod(minutes, 60, &minutes); | ||
| /* XXX ignore sub-minute data, currently not allowed. */ | ||
| assert(seconds == 0); | ||
| if (microseconds != 0) |
There was a problem hiding this comment.
please add { ... } of the if blocks, it's now required by the PEP 7 for new C code.
|
The Travis docs job failed: |
https://bugs.python.org/issue5288