Repository navigation
TZ offset description is unclear in docs #53056
Description
| BPO | 8810 |
|---|---|
| Nosy | @ncoghlan, @abalkin, @benjaminp, @merwok, @matrixise |
| Superseder | |
| Files |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee = None
closed_at = <Date 2016-02-08.04:36:40.077>
created_at = <Date 2010-05-24.16:20:18.764>
labels = ['easy', 'type-feature', 'docs']
title = 'TZ offset description is unclear in docs'
updated_at = <Date 2016-02-08.04:36:40.076>
user = 'https://github.com/abalkin'bugs.python.org fields:
activity = <Date 2016-02-08.04:36:40.076>
actor = 'ncoghlan'
assignee = 'none'
closed = True
closed_date = <Date 2016-02-08.04:36:40.077>
closer = 'ncoghlan'
components = ['Documentation']
creation = <Date 2010-05-24.16:20:18.764>
creator = 'belopolsky'
dependencies = []
files = ['17495', '17496', '26908', '26911']
hgrepos = []
issue_num = 8810
keywords = ['patch', 'easy', 'needs review']
message_count = 18.0
messages = ['106371', '106728', '106729', '106730', '106731', '106732', '106734', '107001', '107002', '107003', '109156', '109157', '168615', '168620', '168623', '221915', '257936', '259815']
nosy_count = 8.0
nosy_names = ['jafo', 'ncoghlan', 'belopolsky', 'benjamin.peterson', 'eric.araujo', 'docs@python', 'bignose', 'matrixise']
pr_nums = []
priority = 'normal'
resolution = 'duplicate'
stage = None
status = 'closed'
superseder = '9305'
type = 'enhancement'
url = 'https://bugs.python.org/issue8810'
versions = ['Python 3.5']
Activity
abalkin commentedon May 24, 2010
From <http://docs.python.org/dev/py3k/library/datetime.html#datetime.tzinfo.utcoffset\>:
"""
tzinfo.utcoffset(self, dt)
"""
This suggests that the return value is an integer representing the number of minutes. It is later explained that in fact "the value returned must be a timedelta object specifying a whole number of minutes", but many users won't read past the first sentence.
I suggest s/in minutes east of UTC/as a timedelta object/. I think "east of UTC" is redundant given the next sentence, "If local time is west of UTC, this should be negative", but that can also be reworded for clarity as "The offset for timezones west of UTC is negative, and for those east of UTC is positive."
jafo commentedon May 29, 2010
In this case, the docs.python.org link you point to seems to be correct, saying that it returns a timedelta. It is the docstring that says it's minutes east of UTC.
I've attached a patch which changes this wording to:
timedelta() showing offset from UTC, with negative for West of UTC
merwok commentedon May 29, 2010
I’m not sure about the “with negative” wording.
jafo commentedon May 29, 2010
Alternately, here is a patch that just takes the docs.python.org description.
jafo commentedon May 29, 2010
Then how about:
timedelta() showing offset from UTC, negative values indicating West of UTC
?
merwok commentedon May 29, 2010
+1 for the last one.
Micro nit: I would not put parentheses when referring to the type (we
talk about “a list”, not “a list()”).
Micro nit: Some languages use case to distinguish direction vs. region,
e.g. in French we have “ouest” and “Ouest”. Does that apply to English?
jafo commentedon May 29, 2010
I'm fine without ().
I thought the direction was generally initial-capped, but I may be wrong there. Let's go with "west".
jafo commentedon Jun 4, 2010
Committed to 2.7 in 81681 and 3.x in 81682.
abalkin commentedon Jun 4, 2010
This issue is specifically about ReST documentation, not the docstring. I explained in the opening comment that 'It is later explained that in fact "the value returned must be a timedelta object specifying a whole number of minutes", but many users won't read past the first sentence.' Ironically, you not noticing that reaffirms my proposition that "many users won't read past the first sentence." :-)
Also, the documentation for tzinfo.dst is similarly unclear with ReST text slightly better than docstring.
Finally, please use rNNNNN form for revisions in commit comments. These are converted to hyperlinks by the tracker. For example, your commit comment would become:
Committed to 2.7 in r81681 and 3.x in r81682.
abalkin commentedon Jun 4, 2010
Sean,
It looks like you committed your first patch rather than your second. Is that what you intended?
Also in msg106734, you agree to change "West" to "west", but committed "West." Note that "west" is correct. In English, the West means the western part of the world while western direction is the west.
Pedantically, the UTC is not a location, so you cannot say "to the west of UTC". Instead, you should say "to the west of the Greenwich Meridian."
Note how RFC 3339 defines the offset:
"""
Numeric offsets are calculated as "local time minus UTC". So the
equivalent time in UTC can be determined by subtracting the offset
from the local time.
"""
I think this is preferable because there are locations such as Spain to the west of Greenwich which use UTC+1.
abalkin commentedon Jul 3, 2010
Benjamin,
Is it too late to do anything about this. Apparently Sean committed wrong patch and as a result 2.7 is about to be released with an error in tzinfo.fromutc docstring. This method is confusing enough without documentation bugs. Would you mind if I revert r81681? This is a docstring only change.
abalkin commentedon Jul 3, 2010
Reverted r81681 and r81682 in r82466 and r82467.
It looks like docstring changes intended for utcoffset() landed in a docstring for fromutc(). Given that we are very close to 2.7 release, I am not attempting to improve the docs - just reverting an obvious error.
bignose commentedon Aug 20, 2012
Here is an updated patch. I examined the implementation in the code for UTC offset and DST handling, and updated the code comments, the docstrings, and the library documentation.
ncoghlan commentedon Aug 20, 2012
It turns out these particular docstrings are duplicated all over the place, as time and datetime both wrap the tzinfo method, and there is both the tzinfo ABC as well as the concrete fixed offset subclasses, and this happens in both C and Python.
Ben's patch currently only covers the docstrings for the Python tzinfo ABC implementation
bignose commentedon Aug 20, 2012
Attached is a patch which is more comprehensive (covering the additional locations pointed out to me by ncoghlan), and also consolidating the details into the library documentation so they're not verbosely repeated in so many places.
I agree with Nick's position that the docstrings are not the place to go into great detail about the significance and background of the issues. So the docstrings in this patch are accurate, but terse; the details are in the library documentation.
abalkin commentedon Jun 29, 2014
Isn't this a duplicate of bpo-9305?
matrixise commentedon Jan 10, 2016
Hi Martin,
Which patch?
Thanks
Stéphane Wirtel - http://wirtel.be - @matrixise
ncoghlan commentedon Feb 8, 2016
I agree it makes sense to merge this and bpo-9305, and since the latter has seen more recent activity, I'll do the merge in that direction.