Author mark.dickinson
Recipients ajaksu2, belopolsky, brett.cannon, daniel.urban, doerwalter, eric.araujo, ggenellina, kawai, l0nwlf, mark.dickinson, pitrou, r.david.murray, rafe, techtonik, tim.peters
Date 2010-06-11.14:23:05
SpamBayes Score 0.000249099
Marked as misclassified No
Message-id <1276266188.13.0.874761314411.issue5094@psf.upfronthosting.co.za>
In-reply-to
Content
Some comments from playing with this patch (without having looked at the implementation):

- As noted above, the 'timezone' class can't be subclassed.  Is this deliberate?  I notice that Brett said "let users subclass as needed to add DST support" in msg107008.

- If I try to do  timezone(timedelta(hours=24)), I get an error message:
"ValueError: offset must be a timedelta between timedelta(1) and -timedelta(1)." and I have to think for a bit to remember that 'timedelta(1)' means 'timedelta(days=1)'.  Any chance of making this more explicit:  e.g. "between timedelta(hours=-24) and timedelta(hours=24)"?

- The existing docs say, at one point: "if utcoffset does not return None,
dst() should not return None either."  And yet it seems that this is exactly what happens for timezone instances:  utcoffset doesn't return None, but dst does.  Was there a reason for the explicit restriction in the docs, and are we sure that that reason is no longer valid?

- I find it strange that mytimezone.utcoffset(1+3j) works;  similarly for tzname and dst.  Perhaps it should be checked at least that the argument is a datetime.  Similarly for tzname and dst.

- And it also seems clunky that an argument *has* to be supplied for utcoffset, tzname and dst, only to be ignored.  Would it be possible to make the argument optional?

- Any chance of a nice __str__ implementation for timezone instances?  (And/or possibly a nice __repr__ as well)?

- The docs for tzname are misleading:  they claim that the default name has the form "UTCsHHMM".  This isn't true for UTC+0, whose name seems to be just "UTC".  It actually wouldn't seem unreasonable to have this print as "UTC+0000", just for consistency (and for ease of parsing for anyone on the receiving end of such a string).  Or the docs could be fixed.

- I'm very confused about utcoffset:  why can't I supply a UTC datetime (i.e. an aware datetime with tzinfo = timezone.utc) to this?  I suspect I'm misunderstanding something here...

- In the docs, replace "timezeone" with "timezone"
History
Date User Action Args
2010-06-11 14:23:08mark.dickinsonsetrecipients: + mark.dickinson, tim.peters, doerwalter, brett.cannon, belopolsky, ggenellina, pitrou, techtonik, ajaksu2, kawai, eric.araujo, r.david.murray, rafe, daniel.urban, l0nwlf
2010-06-11 14:23:08mark.dickinsonsetmessageid: <1276266188.13.0.874761314411.issue5094@psf.upfronthosting.co.za>
2010-06-11 14:23:06mark.dickinsonlinkissue5094 messages
2010-06-11 14:23:05mark.dickinsoncreate