When you call datetime.datetime.utcnow() you get back a naive datetime
object. But why? You asked for UTC as the timezone based on what method
call you made. And UTC is a very concrete timezone that never changes.

It would be nice to have a concrete UTC tzinfo class that utcnow() uses
so that at least those datetime instances are non-naive.

If people have no issues with making this happen I will write the code
for the concrete UTC tzinfo instance and make the appropriate changes to
utcnow().

Brett,
It might be worth to update tzinfo-examples.py to use your concrete UTC
then: 

http://svn.python.org/view/python/trunk/Doc/includes/tzinfo-examples.py?rev=62214&view=markup

On Wed, Jan 28, 2009 at 18:17, Daniel Diniz <report@bugs.python.org> wrote:
>
> Daniel Diniz <ajaksu@gmail.com> added the comment:
>
> Brett,
> It might be worth to update tzinfo-examples.py to use your concrete UTC
> then:

I will if people are generally okay with the idea of adding this class.

I want UTC tzinfo. too.

Seems perfectly reasonable to me.

Please do. The current situation where the doc tells you not to use
"naive" datetime objects but Python gives you no way to do so is awful.

The UTC class have to be converted to C. Can someone write a patch for 
datetimemodule.c (and the doc plus an unit test ;-))?

On Tue, Feb 3, 2009 at 03:28, STINNER Victor <report@bugs.python.org> wrote:
>
> STINNER Victor <victor.stinner@haypocalc.com> added the comment:
>
> The UTC class have to be converted to C.

Yes, the example code is just an example. =)

> Can someone write a patch for
> datetimemodule.c (and the doc plus an unit test ;-))?
>

I might have some people lined up to take this on.

I'm going to attempt to implement this feature.

The patch doesn't include any changes to the documentation.

I thought I had uploaded this last night, apologies.

I am currently doing a review of the patch over at
http://codereview.appspot.com/22042 .

Attaching a new patch by Rafe against Python 2.7. Unfortunately with 2.7 striving for an RC next, this should only target Python 3.2 and not 2.7.

I have two questions about the proposed implementation:

1. Why not follow pytz lead and expose an instance of UTC rather than the UTC class itself?

2. Is there a real need to add a boolean argument to utcnow()?  I think timedelta.now(UTC()) or with utc = UTC() timedelta.now(utc) seems to be a more obvious way to produce TZ aware datetime.

If a singleton instance utc is exposed instead of UTC class, I would suggest to change its repr to 'datetime.utc'.

On the patch itself, datetime_utcnow() is missing an error check for PyObject_IsTrue() return value:

>>> class X:
...    def __nonzero__(self): raise RuntimeError
... 
>>> datetime.utcnow(tz_aware=X())
datetime.datetime(2010, 5, 25, 2, 12, 14, 739720, tzinfo=<datetime.UTC object at 0x1015aab80>)
XXX undetected error
..

Alexander, about 1, that's a pretty good question.  I had originally wanted to do something like that but Brett Cannon at the time thought it was not the right approach.  I don't recall the details.  Maybe Brett can recall.  I think we had that conversation in person and it was a long time ago :(

I had originally thought of doing the class, and then having constants associated with it:

  UTC.UTC0

Eventually if we support multiple timezones:

  UTC.UTC1
  UTC.UTC2
  UTC.UTC_1
  UTC.UTC_2

Well... maybe not given how impossible the naming would be.

I think we also talked about redefining new so that it would always return the same global instance.

On 2, we had discussions about how to pass parameters in to utcnow that we DID record.  When I suggested it, Brett said:

  "...using a boolean flag over an argument is much less error-prone for a developer from passing in the wrong timezone object; passing in something other than an instance of UTC would just be stupid so we should make sure the developer isn't stupid. =)"

We didn't do a singleton because I don't like singletons. =) Plus they muck with isinstance/issubclass if you don't expose the class. Basically there is no need to have it be a singleton, so why bother?

And Rafe is right: since utcnow() already exists, why not take advantage of the method? Yes, you could manually call now() with a UTC object, but people are going to notice the utcnow() method and want to use it, so we should make it easy to use the new UTC object on utcnow(). Plus it has the added benefit of providing a transition plan to make utcnow() always return a timezone-aware datetime object.

On Mon, May 24, 2010 at 11:06 PM, Rafe Kaplan <report@bugs.python.org> wrote:
..
> On 2, we had discussions about how to pass parameters in to utcnow that we DID record.  When I
> suggested it, Brett said:
>
>  "...using a boolean flag over an argument is much less error-prone for a developer from passing in the wrong
> timezone object; passing in something other than an instance of UTC would just be stupid so we should make
> sure the developer isn't stupid. =)"

Well, I respectfully disagree.  This advise seems to be placing
convenience of the writer of the code over that of the reader.
Imagine encountering an expression datetime.utcnow(True) allowed by
your current patch and trying to figure out what it means.  This can
be improved by making tz_aware a keyword only argument, but in that
case a separate datetime.tz_aware_utcnow() function seems like a
better choice.

Note that I am not suggesting passing anything to utcnow().  I would
either leave it unchanged or make it always return aware datetime
instances.  (Note that with singleton UTC timezone naive datetime
instances can be deprecated with no performance penalty.)

> We didn't do a singleton because I don't like singletons. =) Plus they
> muck with isinstance/issubclass if you don't expose the class.

I am not sure what you mean by "muck with."  Why would anyone want to subclass UTC?

> Basically there is no need to have it be a singleton, so why bother?

There are several advantages of having all datetime instances with tzinfo=UTC() referring to the same instance:

1. Comparison (and I believe subtraction) of aware datetime instances bypasses calculation of utcoffset if their tzinfo attributes refer to the same object.

2. With the current patch,

>>> set(UTC() for i in range(3))
set([<datetime.UTC object at 0x1015aac80>, <datetime.UTC object at 0x1015aad00>, <datetime.UTC object at 0x101a0e040>])

I don't think this is intended.  Basically UTC() instances cannot be meaningfully compared or used as dictionary or set keys.  You can fix it by providing custom __eq__ and __hash__, but this problem simply goes away if a singleton is exposed instead.

3. now(utc) is slightly more readable than now(UTC())

4. Singleton utc is familiar to pytz users.

> Note that I am not suggesting passing anything to utcnow().  I would
> either leave it unchanged or make it always return aware datetime
> instances.

The latter would break compatibility, though (especially given how
comparison between "naive" and "aware" datetimes raises an error...).

I also agree with Brett that a singleton looks rather unnecessary (it
also look quite C++/Java-esque to me).

On the subject of the patch:
- Alexander spotted the PyObject_IsTrue() issue
- you should never use tabs (anymore), only 4 spaces
- lines like:
    self.assertTrue(isinstance(now.tzinfo, UTC))
can be rewritten as
    self.assertIsInstance(now.tzinfo, UTC)
- here, I'm not sure why you're using "UTC" as the error message
(copy/paste error?):
    self.assertEquals(None, meth(False).tzinfo, UTC)
- you might make the UTC class subclassable, although I'm not sure
there's any point

On Tue, May 25, 2010 at 5:45 AM, Antoine Pitrou <report@bugs.python.org> wrote:
..
> I also agree with Brett that a singleton looks rather unnecessary (it
> also look quite C++/Java-esque to me).
>

I still don't understand your aversion to singletons and you did not
address any of the advantages that I listed in my previous comment.  I
don't think singletons are foreign to Python: after all we write None
rather than NoneType() .

We can reach a middle ground by interning UTC instances behind the
scenes so that UTC() is UTC() will always be true.  This will address
most of the issues that I raised and utc = datetime.UTC() is simple
enough to write as long as you don't have to worry about sharing utc
instance between modules.

On Mon, May 24, 2010 at 11:06 PM, Rafe Kaplan <report@bugs.python.org> wrote:
..
> I had originally thought of doing the class, and then having constants associated with it:
>
>  UTC.UTC0
>
> Eventually if we support multiple timezones:
>
>  UTC.UTC1
..
>
> Well... maybe not given how impossible the naming would be.
..

I actually like your original idea.  It seems wasteful to create a
concrete timezone class in a C module and only use it for a single
timezone.  FixedOffset class in tzinfo-examples.py is only slightly
more complicated than UTC class and as explained in the comment above
it, "FixedOffset(0, "UTC") is a different way to build a UTC tzinfo
object.  FixedOffset objects can then be used to produce aware
datetime instances from strptime.  (See issue6641.)  I would only
define utc = FixedOffset(0, "UTC") instance and make name argument to
FixedOffset optional defaulting to UTC(+/-)hhmm.

The singleton dislike from Antoine and me is that they are generally just not liked in the stdlib. None/True/False are special cases because they are syntax, so having ``None is None`` ever not work would just be weird. Otherwise singletons are unnecessary in Python. Just look through the stdlib and you will find very few singletons as they are generally considered bad. Having to write a custom __eq__ or __hash__ is just part of being explicit. And trying to make a factory function that always returns the same instance is not a solution either. I understand pytz might use them, but this is the stdlib, so we need to go with what we consider best practice for Python since it will lead to much more use than pytz gets.

Now if a simple FixedOffsetTimeZone class was added and we just pre-populated the datetime module with a utc attribute that contained an instance of that class set to the proper values for UTC, that I could support without controversy. That would get you your "singleton" by reliably using the same instance without having to try to hack in singleton support.

..
Thanks for the explanation.  I realize that I should not have used the
s-word. :-)  In fact I only wanted a module level constant utc = UTC()
and did not care much about other UTC class instances and whether any
are permitted or easy to create.

Well, the datetime module is not exactly the place you want to start
if you want to lead anyone to best Python practices. :-) (Just think
of datetime subclassing from date!)

> Now if a simple FixedOffsetTimeZone class was added and we just pre-populated the datetime module with a utc attribute that contained an
> instance of that class set to the proper values for UTC, that I could support without controversy.

This is exactly my preferred solution.

"Note that I am not suggesting passing anything to utcnow().  I would
either leave it unchanged or make it always return aware datetime
instances."

FYI, all other issues aside, having utcnow() (with no parameters) return a tzaware instance will introduce backward compatibility problems.  As it is, users are not expecting utcnow to return a date-time with any tzinfo.

Roundup bug bites again.  Reposting via web:

-----
On Tue, May 25, 2010 at 5:35 PM, Brett Cannon <report@bugs.python.org> wrote:
>
> The singleton dislike from Antoine and me is that they are generally just not liked in the stdlib.
..
Thanks for the explanation.  I realize that I should not have used the
s-word. :-)  In fact I only wanted a module level constant utc = UTC()
and did not care much about other UTC class instances and whether any
are permitted or easy to create.

> .. so we need to go with what we consider best practice for Python since it will lead to much more use than pytz gets.
>
Well, the datetime module is not exactly the place you want to start
if you want to lead anyone to best Python practices. :-) (Just think
of datetime subclassing from date!)

> Now if a simple FixedOffsetTimeZone class was added and we just pre-populated the datetime module with a utc attribute that contained an
> instance of that class set to the proper values for UTC, that I could support without controversy.

This is exactly my preferred solution.

Note that Brett has already mentioned backward compatibility issues, but suggested that "[adding tz_aware argument may provide] a transition plan to make utcnow() always return a timezone-aware datetime object." [msg106413]

I would say, lets leave utcnow() alone.  It is ugly enough without a boolean argument.  I don't see how datetime.now(utc) can be too error prone.  I think Brett's comment about a stupid developer was about passing tzinfo instead of bool to utcnow() and that I agree makes no sense.

If we don't modify utcnow (and similar UTC methods) to take a flag saying to use the utc instance, then the methods should at least get deprecated with a message saying that people should be using ``now(utc)``, etc.

Brett: "[utcnow] should at least get deprecated with a message saying that people should be using ``now(utc)``"

Yes, I believe all utcxxx methods of datetime are a kludge due to the lack of concrete UTC tzinfo:

utcfromtimestamp() -> fromtimestamp(utc)
t.utctimetuple() -> t.replace(tzinfo=utc).timetuple()

OK, it looks like we are reaching consensus here on several points:

1. Implement FixedOffsetTimezone
2. Provide a utc attribute on the datetime module that is set to ``FixedOffsetTimezone(0, "UTC")``
3. Deprecate the various utc* methods with messages pointing out how to use the new utc instance instead of the method

If this seems reasonable, then I see two questions to answer.

First is how long to do the deprecations. I say remove in Python 3.5. Existing for three versions is six more years of usage from the time of 3.2's release to that of 3.6. Plus it is easy to be backwards-compatible by showing in the docs how to create one's own UTC class.

The second is whether we should take this opportunity to fix datetime being a C extension module exclusively. I know PyPy has their own pure Python version of datetime that they plan to eventually contribute. We might as well use this as the chance to create Lib/datetime.py and have that conditionally import _datetimemodule.c (see the heapq module on how to handle this kind of situation). That way PyPy can eventually just drop their code into datetime.py. Biggest issue will be extension modules wanting to use the C extension API, but since this is new stuff it shouldn't affect them except for the module renaming.

I have no opinion on the first question.  I would be fine with a simple "soft" deprecation where we simply add a note in documentation warning that these methods create naive datetime instances and it is preferable to use aware variants produced by meth(utc).  On the other hand eventually removing these methods will make maintenance easier.  Sorry I cannot offer more help with this decision.

With respect to the second question, I would be against mixed C/Python implementation.  I would also like to see C API to the new concrete tzinfo  classes.

The stated long-term goal of the stdlib is to minimize the C extension modules to only those that have to be written in C (modules can still have performance enhancing extension back-ends). Since datetime does not meet that requirement it's not a matter of "if" but "when" datetime will get a pure Python version and use the extension code only for performance.

If someone wants to implement the C code for a tzinfo concrete class that we are talking about, that's fine. But that will not prevent datetime from getting a pure Python version at some point.

> The second is whether we should take this opportunity to fix datetime
> being a C extension module exclusively. I know PyPy has their own pure
> Python version of datetime that they plan to eventually contribute. We 
> might as well use this as the chance to create Lib/datetime.py and have 
> that conditionally import _datetimemodule.c (see the heapq module on 
> how to handle this kind of situation).

Of we could let PyPy contribute their own version when they want, after all.
I think additions to the datetime API are good in themselves, and we shouldn't make them depend on some mythical refactoring of the code into a separate pure Python module ;)

PyPy has said over the years they plan to commit their version of datetime, they just need to get around to it. I just figured that we could use this opportunity to prepare for it. But if people want to do the C version first, that's fine as they will be the ones writing the patch. I just thought that if someone would rather write a patch to create datetime.py now along with a pure Python version of the proposed class they could.

Here is my first attempt to implement fixed offset timezone type.  The patch is based on Brett's next-patch.txt and while I changed the type name from datetime.UTC to datetime.timezone, I did not change the name of the related C structures.  I would like to ask for comments on the following questions:

1. How to call the new type?  I like "timezone" because it is likely to be the only concrete tzinfo subclass in datetime module, so we don't really need to call it fixedoffsetfromutctimezone.

2. Do we want to add a dst indicator and altname attributes? I would say: no. I would rather treat DST as a different fixed offset timezone.

3. I am not quite happy about having to specify offset in minutes.  I think timezone(hours[, minutes]) may be clearer.  Alternatively we may just take offset as a timedelta.  Note issue5288.  There is some interest in supporting sub-minute timezones.

4. I have fixed a reference leak in utcnow, but I am still against giving it tz_aware argument.

My thoughts on Alexander's questions:

1. Call it FixedTimezone or something (remember it has to be CapWords). Calling it simply Timezone does not convey the fact that DST is not supported and people might naively think it will. Its limited abilities should be portrayed in the name.

2. Keep the class dead-simple. The primary motivator is to support UTC, maybe the %z directive for strptime. Otherwise everything else should be left out of the stdlib and let third-parties manage it as they will be the ones that need to manage the bazillion timezone instances they need. We don't need to dictate an interface to them.

3. Taking a timedelta makes sense since the class represents the fixed time offset from UTC.

As for the tz_aware argument to utcnow and friends, I am fine with letting go of them if we have a utc attribute on datetime and we simply document that to get a UTC-aware value do ``now(datetime.utc)`` and consider deprecating utcnow.

On Wed, Jun 2, 2010 at 5:24 PM, Brett Cannon <report@bugs.python.org> wrote:
..
> 1. Call it FixedTimezone or something (remember it has to be CapWords).

I thought consistency within module trumps PEP 8 naming standards.
The datetime module (for better or worse) uses lowercase names for its
types: date, time, datetime, tzinfo.  Shouldn't the new type follow
the suit? (This will also avoid a source of typos TimeZone vs.
TimeZone).

I don't like "fixed timezone" - it is not clear what it is fixed:
offset, geographical location or historical set of rules.  I think we
should promote the notion that timezone is just an offset.  EST is -5
hours, EDT is -4.  New York uses EST in winter and EDT in summer.   A
zoneinfo database (external to python) is a mapping from place and
time to timezone.

Forgot about datetime breaking the PEP 8 rules. You're right, consistency wins.

As for fixedtimezone being odd, that's why my mind went with FixedOffsetTimezone to start, but that doesn't go with the naming of the module, and fixedoffsettimezone is just hard to read.

As long as the documentation is VERY clear that the timezone class only supports a fixed offset from UTC and nothing else I can live with the "timezone".

I am attaching the next installment of the datetime.timezone class implementation.

Here I add ``utc`` class attribute to timezone.  I decided to place it in class rather than module namespace because this seems to be more inline with how datetime module defines particular instances of its classes such as ``min``, ``max`` and ``resolution``. I also feel that writing ``timezone.utc`` makes it clearer that its is an instance of ``timezone`` class while ``datetime.UTC`` or simply ``UTC`` is more ambiguous.

I also changed ``timezone`` constructor to interpret int or float offset as number of hours and accept arbitrary timedelta between timedelta(hours=-12) and timedelta(hours=12).  The rationale is that most common timezones have offsets at whole hours and less common but existing timezones use 1/2 or 1/4 hour offsets and thus can be specified as a binary float without any issue.

I've added tests and some preliminary documentation.

I don't think people would get confused as to what datetime.utc was, but as you pointed out, Alexander, the module seems to like class attributes so timezone.utc is fine.

As for the float/int argument, I personally am wary of it. Since the timedelta constructor accepts hours as a keyword argument, I don't see the benefit of having to support both timedeltas and int/floats. And I can see someone messing up and putting in a float that is not perfectly representable and getting upset at odd behavior. I say keep it simple and just accept timedeltas for now. If there really is demand for accepting integers in the constructor than it can be added without backwards-compatibility issues. Better to keep the API small and expand later than make it too big to start and being burdened with extraneous API stuff.

> accept arbitrary timedelta between timedelta(hours=-12) and
> timedelta(hours=12)

Aren't there valid timezones that are offset by more than 12 hours from UTC?

On Thu, Jun 3, 2010 at 3:19 PM, Mark Dickinson <report@bugs.python.org> wrote:
..
> Aren't there valid timezones that are offset by more than 12 hours from UTC?

I am not sure.   At this stage treat 12 as a placeholder for whatever
the relevant standard says.  I've seen suggestions that the range
should be (-24, 24) excluding ends
<http://www.zope.org/Members/fdrake/DateTimeWiki/BasicDesign> and
[-14, 14] <http://msdn.microsoft.com/en-us/library/bb630289.aspx>.

On Thu, Jun 3, 2010 at 3:41 PM, Alexander Belopolsky
<report@bugs.python.org> wrote:
..
> I am not sure.   At this stage treat 12 as a placeholder for whatever
> the relevant standard says.

Believe it or not, at least one standard, RFC 2822, allows any offset
representable as HHMM: "the zone MUST be within the range -9959
through +9959" <http://tools.ietf.org/html/rfc2822.html>.

I am inclined to simply remove any range checking and allow arbitrary
timedelta as an offset.

On Thu, Jun 3, 2010 at 3:15 PM, Brett Cannon <report@bugs.python.org> wrote:
..
> As for the float/int argument, I personally am wary of it. Since the
> timedelta constructor accepts hours as a keyword argument, I don't
> see the benefit of having to support both timedeltas and int/floats.

To my taste, timedelta(hours=-5) is just too verbose.  It also forces
you to import timedelta in a module that may otherwise not need it.

> And I can see someone messing up and putting in a float that is not > perfectly representable and getting upset at odd behavior.

Since timedelta accepts floats for hours, this argument does not
really hold.  Also, with timedelta correctly rounding to the nearest
microsecond, it is really hard to mess up with binary vs. decimal
representation.

> I say keep it simple and just accept timedeltas for now. If there
> really is demand for accepting integers in the constructor than it can > be added without backwards-compatibility issues. Better to keep the > API small and expand later than make it too big to start and being
> burdened with extraneous API stuff.
>

This a valid argument.  since it is easier to remove code than to add,
I'll keep int/float support in the patch while we are discussing the
design, but I am ready to remove it.

I am having second thoughts about dst indicator.  I wrote:
"""
2. Do we want to add a dst indicator and altname attributes? I would say: no. I would rather treat DST as a different fixed offset timezone.
"""

and Brett responded: 
"""
2. Keep the class dead-simple. The primary motivator is to support UTC, maybe the %z directive for strptime. Otherwise everything else should be left out of the stdlib and let third-parties manage it as they will be the ones that need to manage the bazillion timezone instances they need. We don't need to dictate an interface to them.
"""

Now note, that with fixed offset timezone class, it is possible to produce aware local times as follows:

from datetime import datetime, timezone, timedelta
import time
EPOCH = datetime(1970, 1, 1)
def localtime(utctime=None):
    if utctime is None:
        tm = time.localtime()
    else:
        seconds = (utctime - EPOCH).total_seconds()
        tm = time.localtime(seconds)

    tz = (timezone(timedelta(seconds=-time.altzone), time.tzname[1])
          if tm.tm_isdst else
          timezone(timedelta(seconds=-time.timezone), time.tzname[0]))
    return datetime(*tm[:6], tzinfo=tz)

(see also attached localtime.py)

The problem with the above implementation is that t.timetuple().tm_isdst will always be 0 if t is produced by localtime().

I don't think adding fixed dst offset is much of complication.  We already need to override the tzinfo.dst method and if we only allow timedeltas as offset and dst arguments to constructor, the constructor code will be extremely simple.

So you want a third argument that lets you flag if the timezone is DST or not? I still don't think that will be necessary. If people want to add that they can very easily subclass the timezone class and add support for it. This class should be focused on providing a UTC instance and anything needed for a %z directive in strptime, nothing more. Anything fancier can be handled by libraries like pytz as they need it. Once again, keep the APi as simple as possible and add features as needed. I know how tempting it it is to design upfront, but just trust me, Alexander, we will all get burned for it later.

> So you want a third argument that lets you flag if the timezone is DST or not?

The third argument is not a flag, it is a timedelta just like the offset.  I am attaching a python prototype for clarity. (See datetimeex.py.)

Conceptually, a 3-argument timezone is very simple: tzinfo defines three abstract methods: utcoffset(..), tzname(..), and dts(..).  The proposed concrete implementation lets allows the user to provide constant values to be returned from each of these methods.

I'm still leary of supporting any form of DST. A proper DST implementation would need to have some conditional code to account for the datetime object passed into dst, and yet the version you have prototyped doesn't handle it. So a proper timezone supporting DST would still need to subclass any concrete class.

I still say keep it as simple as possible and let users subclass as needed to add DST support. Subclassing __init__ and dst() is not difficult if you want to add proper DST support, especially if dst() is set to return timdelta(0) and utcoffset() always returns CONSTANT + self.dst().

And just to mention it, the instance attributes you had in your example, Alexander, were not "private". For any final code, make sure you make them private else you are asking for trouble from people starting to rely on those attributes.

On Thu, Jun 3, 2010 at 3:19 PM, Mark Dickinson <report@bugs.python.org> wrote:
..
> Aren't there valid timezones that are offset by more than 12 hours from UTC?

Indeed, Christmas Island uses UTC+14. (http://en.wikipedia.org/wiki/Kiritimati).

The most western timezone seems to be UTC-12 used on two uninhabited
islands. <http://en.wikipedia.org/wiki/Time_zone#Time_zone_as_offsets_from_UTC>

The tzinfo specification requires [-24, 24] hours range:

"""
.. the value returned must be a timedelta object specifying a whole
number of minutes in the range -1439 to 1439 inclusive (1440 = 24*60;
the magnitude of the offset must be less than one day).
""" -- http://docs.python.org/dev/py3k/library/datetime.html#datetime.tzinfo.utcoffset

I am torn between two options with a slight preference for the first:

1. Don't do any checking in the constructor and allow any timedelta
used as an offset.  This is the simplest to implement and most future
proof.  For example, it may be desirable to extend [-24, 24] to at
least [-99, 99] to allow round-tripping of compliant RFC 3339
timestamps.  (Note that I am not suggesting that real life more than a
day offsets are possible, but once a standard allows impossible
values, people tend to abuse them as special markers in their data.)

2. Require [-24, 24] hours range.  This is the letter of the current
tzinfo.utcoffset() definition.

Opinions?

What do you think

Merging in issue7584 nosy list.

> I'm still leary of supporting any form of DST. A proper DST
> implementation would need to have some conditional code to
> account for the datetime object passed into dst, and yet the
> version you have prototyped doesn't handle it.

No, any tzinfo implementation where utcoffset(dt) depends on dt is broken because once utcoffset starts to vary with time you can no longer determine a point in time from the local time alone. (In theory, a continuously increasing or decreasing in time utcoffset is an exception to this rule, but there is no practical use for those.)

This limitation is admitted in datetime.tzinfo documentation:

"""
Note that there are unavoidable subtleties twice per year in a tzinfo subclass accounting for both standard and daylight time, at the DST transition points. ... Applications that can’t bear such ambiguities should avoid using hybrid tzinfo subclasses; there are no ambiguities when using UTC, or any other fixed-offset tzinfo subclass (such as a class representing only EST (fixed offset -5 hours), or only EDT (fixed offset -4 hours)).
"""

(See three paragraphs above http://docs.python.org/dev/py3k/library/datetime.html#strftime-and-strptime-behavior)

In pytz, <http://pytz.sourceforge.net/#tzinfo-api>, tzinfo API is extended to add an is_dst flag to utcoffset(), tzname(), and dst() methods, but since datetime objects do not carry this flag, it is impossible for datetime module to pass this flag to timezone within datetime.datetime methods and datetime module does not know about this flag to begin with.

To add insult to injury, the extended API still does not solve all the problems: <http://pytz.sourceforge.net/#problems-with-localtime>.

> So a proper timezone supporting DST would still need to subclass
> any concrete class.

No, as I explained above, it is not possible to implement a "proper timezone."  I believe most of the frustration with the current tzinfo API stems from the fact that it is not implementable.   The correct interface to a timezone database should provide a mapping from (universal time, geographical location) to civil time there and then.  A common name for the timezone in use and information about DST being in effect is useful for interoperability but not strictly required.

This is what I implemented in my localtime() prototype in localtime.py (loosing DST information) and datetimeex.py (interoperable with POSIX timetupe based interfaces and pytz extended API).

Note that on systems supporting extended tm structure (with tm_zone and tm_gmtoff fields), it is possible to implement localtime() which will take advantage of the full historical timezone information available on the system.

For the allowable range, follow the datetime docs as someone might be relying on that specification already.

As for the ongoing DST debate, it seems we either need to say that since we cannot properly support all possible datetimes properly we should simply not even try, fixed offset or not, or we provide a class that gives the proper UTC offset, but in no way adjusts itself as people might expect or want. I'm arguing for the former, Alexander wants the latter.

I still stand by my argument that it is not needed for the two use cases that we concretely have in the stdlib for a timezone class: a UTC instance and %z directive in strptime. Unless there is some way for the %z directive to specify that it is actually DST, I still think the functionality of the class should be kept to a functional minimum for our needs and let people needing more support, including fixed offset DST, decide how they want to handle it. I can still see a naive user thinking that DST is the same around the world and being taken by surprise when thing don't adjust accordingly by the timezone when he does ``timezone("PDT", 7, dst=True)``. Plus giving people any semblance of a DST-supporting timezone class is just going to lead for more calls of the stdlib to include concrete timezone instances that do manage DST.

Unless other people step forward to debate this we probably are not going to reach consensus without going to python-dev to see what others think.

On Fri, Jun 4, 2010 at 4:09 PM, Brett Cannon <report@bugs.python.org> wrote:
..
> For the allowable range, follow the datetime docs as someone might be relying on that specification already.
>
Will do.  I think it is as simple simple s/12/24/ in two places of the
current patch, but I will check and make sure it is tested by
unittests.

> As for the ongoing DST debate, it seems we either need to say that since we cannot properly support all possible
> datetimes properly we should simply not even try, fixed offset or not, or we provide a class that gives the proper UTC
> offset, but in no way adjusts itself as people might expect or want. I'm arguing for the former, Alexander wants the latter.
>

I disagree with your premise that "we cannot properly support all
possible datetimes." If issue1647654 patch is applied, I can rewrite
datetimeex.py so that print(localtime()) is consistent with the system
date utility on Linux/MacOS/BSD for all times.  This however can be
achieved without DST offset in timezone object.

The only issue is what time.strftime("%Z", localtime().timetuple())
will print.  Recall that tm_isdst (the last entry in timetuple) is set
according to what dst() returns.  If dst() is None,  tm_isdst = -1, if
dst() is timedelta(0),  tm_isdst = 0 and if dst() is non-zero
timedelta, tm_isdst = 1.  In these three cases, %Z will be formated as
empty string, time.tzname[0] ('EST' in New York) and time.tzname[1]
('EDT' in New York) respectively.

I would really like to get localtime() in datetime library at the next
step. In the distant future, datetime.now() can be redefined to return
aware local time.  If this happens, we need to decide what
localtime().dst() should return.  With the current patch, it will
return timedelta(0) which is wrong 50% of the time.  A correct
alternative is to return None, but this will be rather odd for
timezone.utc.

It appears that in order to implement dst() correctly to the letter of
tzinfo specification there are really two choices:

1.  Have separate classes: UTC and FixedOffsetTimezone with different
dst().  UTC().dst(dt) -> timedelta(0) and
FixedOffsetTimezone(offset).dst(dt) -> None.

2. Have a single class with dst(dt) returning fixed stored value.

I think #2 clearly wins on simplicity.

> I still stand by my argument that it is not needed for the two use cases that we concretely have in the stdlib for a
> timezone class: a UTC instance and %z directive in strptime.

I agree.  The third member is needed to support aware local time,
which is the third use case.

> ..
> I can still see a naive user thinking that DST is the same around the world and being taken
> by surprise when thing don't adjust accordingly by the timezone when he does ``timezone("PDT", 7, dst=True)``.

The actual syntax would be PDT = timezone(timedelta(hours=7), "PDT",
dst=timedelta(hours=1)) (or -1 - I have to check).  Is the surprise
you have in mind that PDT.utcoffset(dt) is the same for dt in June and
January? I agree this is not ideal and I would much rather not allow
timezone.utcoffset() to take an argument, but this is not possible
within the tzinfo design.  Best we can do is to make the argument to
utcoffset() optional and naive users will not even know that they can
pass time value to the timezone methods.

I think this is still less evil than to have an aware datetime t with
t.tzname() -> 'PDT' and time.strftime('%Z', t.timetuple()) -> 'PST'.

> Plus giving people any semblance of a DST-supporting timezone class is just going to lead for more calls of the
> stdlib to include concrete timezone instances that do manage DST.
>

Again, managing DST is to use different timezone instances for
different real times.  It is not hard to do that.  Implementing
time-dependent utcoffset(dt) in a concrete tzinfo subclass is
impossible unambiguously.  In the long term we should be looking to
deprecate the time argument to tzinfo methods.

> Unless other people step forward to debate this we probably are not going to reach consensus
> without going to python-dev to see what others think.

Given the history of such discussions, I think we should reach
consensus here first.  I don't mind starting with a two-component
timezone class and dst() returning None for all instances including
timezone.utc.  I still believe the cost of supporting DST is really
small and adding it now is easier than in the future.

On Fri, Jun 4, 2010 at 15:17, Alexander Belopolsky
<report@bugs.python.org> wrote:
>
> Alexander Belopolsky <belopolsky@users.sourceforge.net> added the comment:
>
> On Fri, Jun 4, 2010 at 4:09 PM, Brett Cannon <report@bugs.python.org> wrote:
> ..
>> For the allowable range, follow the datetime docs as someone might be relying on that specification already.
>>
> Will do.  I think it is as simple simple s/12/24/ in two places of the
> current patch, but I will check and make sure it is tested by
> unittests.

Great!

>
>> As for the ongoing DST debate, it seems we either need to say that since we cannot properly support all possible
>> datetimes properly we should simply not even try, fixed offset or not, or we provide a class that gives the proper UTC
>> offset, but in no way adjusts itself as people might expect or want. I'm arguing for the former, Alexander wants the latter.
>>
>
> I disagree with your premise that "we cannot properly support all
> possible datetimes." If issue1647654 patch is applied, I can rewrite
> datetimeex.py so that print(localtime()) is consistent with the system
> date utility on Linux/MacOS/BSD for all times.  This however can be
> achieved without DST offset in timezone object.

Well, I think the time module needs to be slowly phased out in
preference for datetime and we stop monkey-patching the time tuple
over and over again. The datetime module provides a much nicer
interface and TOOWTDI, but that's a long term goal.

>
> The only issue is what time.strftime("%Z", localtime().timetuple())
> will print.  Recall that tm_isdst (the last entry in timetuple) is set
> according to what dst() returns.  If dst() is None,  tm_isdst = -1, if
> dst() is timedelta(0),  tm_isdst = 0 and if dst() is non-zero
> timedelta, tm_isdst = 1.  In these three cases, %Z will be formated as
> empty string, time.tzname[0] ('EST' in New York) and time.tzname[1]
> ('EDT' in New York) respectively.
>
> I would really like to get localtime() in datetime library at the next
> step. In the distant future, datetime.now() can be redefined to return
> aware local time.  If this happens, we need to decide what
> localtime().dst() should return.  With the current patch, it will
> return timedelta(0) which is wrong 50% of the time.  A correct
> alternative is to return None, but this will be rather odd for
> timezone.utc.

True.

>
> It appears that in order to implement dst() correctly to the letter of
> tzinfo specification there are really two choices:
>
> 1.  Have separate classes: UTC and FixedOffsetTimezone with different
> dst().  UTC().dst(dt) -> timedelta(0) and
> FixedOffsetTimezone(offset).dst(dt) -> None.
>
> 2. Have a single class with dst(dt) returning fixed stored value.
>
> I think #2 clearly wins on simplicity.

Or #3: We have a UTC class as originally proposed way back when. We
can even make all the methods staticmethods so that I get my desire to
not require instantiating an instance while you do get your
pseudo-singleton you originally wanted.

>
>> I still stand by my argument that it is not needed for the two use cases that we concretely have in the stdlib for a
>> timezone class: a UTC instance and %z directive in strptime.
>
> I agree.  The third member is needed to support aware local time,
> which is the third use case.

Sure, but we are not even there yet to consider that use case are we?

>
>> ..
>> I can still see a naive user thinking that DST is the same around the world and being taken
>> by surprise when thing don't adjust accordingly by the timezone when he does ``timezone("PDT", 7, dst=True)``.
>
> The actual syntax would be PDT = timezone(timedelta(hours=7), "PDT",
> dst=timedelta(hours=1)) (or -1 - I have to check).

Yes, I was typing from the cuff.

>  Is the surprise
> you have in mind that PDT.utcoffset(dt) is the same for dt in June and
> January?

Exactly.

> I agree this is not ideal and I would much rather not allow
> timezone.utcoffset() to take an argument, but this is not possible
> within the tzinfo design.  Best we can do is to make the argument to
> utcoffset() optional and naive users will not even know that they can
> pass time value to the timezone methods.

This is all starting to feel like the entire tzinfo ABC needs to be
re-thought and either expanded upon or replaced.

>
> I think this is still less evil than to have an aware datetime t with
> t.tzname() -> 'PDT' and time.strftime('%Z', t.timetuple()) -> 'PST'.
>
>> Plus giving people any semblance of a DST-supporting timezone class is just going to lead for more calls of the
>> stdlib to include concrete timezone instances that do manage DST.
>>
>
> Again, managing DST is to use different timezone instances for
> different real times.  It is not hard to do that.  Implementing
> time-dependent utcoffset(dt) in a concrete tzinfo subclass is
> impossible unambiguously.  In the long term we should be looking to
> deprecate the time argument to tzinfo methods.

This is why I hate datetime stuff. =)

>
>> Unless other people step forward to debate this we probably are not going to reach consensus
>> without going to python-dev to see what others think.
>
> Given the history of such discussions, I think we should reach
> consensus here first.  I don't mind starting with a two-component
> timezone class and dst() returning None for all instances including
> timezone.utc.  I still believe the cost of supporting DST is really
> small and adding it now is easier than in the future.

So how about this: we implement a UTC class now which contains only
staticmethods to get you your singleton and me not having to
instantiate the class. Then you take your time in possibly expanding
the tzinfo class or coming up with a new ABC which replaces tzinfo
that fixes all of these crazy problems. I have been arguing from a
code maintainability and naive user POV, you from an experienced
datetime POV. Lets solve my POV now and just start the ball rolling in
solving your POV properly long-term instead of trying to shoe-horn
known issues into pre-existing stuff. People complain about datetime
enough, so let's take the time to fix it. The UTC class can be a
stop-gap until datetime gets fixed (which could be in 3.2 if stuff
happened really rapidly and the UTC solution can be ripped out; it's
just code).

On Fri, Jun 4, 2010 at 11:09 PM, Brett Cannon <report@bugs.python.org> wrote:
> Unless other people step forward to debate this we probably are not going to reach consensus without going to python-dev to see what others think.

I would really like to see:
1. a good definition of the problem
2. summary of research
3. possible solutions

The amount of text generated in bugtracker is already overwhelming for
others to plug in. I remember that my thread about datetime TZ was
also hijacked into the debate that we can not ship 3rd party TZ DB
with Python, so I can't even reference it.

I am attaching the next version of my patch and changing priority to high because it appears that several issues depend on resolution of this one.

Here is the summary of changes since issue5094a.diff:

1. The constructor now accepts only whole number of minutes in [-23:59, 23:59] range.

2. Removed tz_aware option to utcnow().

3. Moved struct PyDateTime_TimeZone definition to .c file. This effectively makes the actual definition private so that it can be changed in the future if desired without breaking C API.

4. Removed checks that the argument passed to tzinfo methods is a timedelta.  The tzinfo spec requires that None is accepted.  Since the argument is ignored, it is wasteful to check its type.

5. Make dst() return None as recommended by tzinfo documentation when DST information is not provided.

Brett, is this close to a compromise that we can agree to or is it time to start writing a PEP? :-)

PS: The latest patch does not include doc changes.

Isn't it possible, that in the issue5094b.diff patch, in the new_timezone_ex function, in this part:

self = (PyDateTime_TimeZone *)type->tp_alloc(type, 0);
if (self == NULL)
    return NULL;

should be a Py_DECREF(offset) call?

I mean like:

self = (PyDateTime_TimeZone *)type->tp_alloc(type, 0);
if (self == NULL) {
    Py_DECREF(offset);
    return NULL;
}

I think the refcount of offset has always been incremented, so in case of an error, it should be decremented (as in the other error handling parts of the function).

Good catch, Daniel!

Fixed in issue5094c.diff.

Put yourself down as the maintainer of datetime in Misc/maintainers.rst and you have a deal, Alexander! =)

Readying the patch for commit.  Added documentation, tests and implementation of fromutc() method that does not need a dst().

In Doc/library/datetime.rst, in the documentation of datetime.utcnow this part: ".. versionchanged:: 2.7" probably should be ".. versionchanged:: 3.2".

Thanks, Daniel.  This was a leftover from the previous patch.  The latest patch does not actually change utcnow().  Instead the users should call datetime.now(timezone.utc). Fixed in issue5094d1.diff.

I'm not sure if I missed this in the earlier discussion:  is there a reason to prevent subclassing of timezone?

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"

One more: there's a footnote marked in the docs (line 36 of datetime.rst), but no corresponding footnote as far as I can see.

And another minor doc issue:  the docs still say:

"The datetime module does not supply any concrete subclasses of tzinfo."

On Fri, Jun 11, 2010 at 10:23 AM, Mark Dickinson <report@bugs.python.org> wrote:
..
> Some comments from playing with this patch (without having looked at the implementation):
>
thank you very much for your comments.  As we are fine-tuning the
timezone class, do you think it would be helpful to go back to Brett's
suggestion and add datetime.py with just from _datetime import * and
keep parallel Python and C implementations of timezone class only
until full datetime.py is available?

I was deliberate in a sense that I thought about it, but there was no
substantial discussion.  Original patch did not allow subclassing of
the UTC class and I thought it was right.  (See msg106415 and
msg106422.) subclassing FixedOffsetTimeZone makes more sense, but I
was hoping to keep timezone complete and eventually stop supporting
tzinfo API (not allow passing datetime in the methods.)  More on this
below.  I would very much prefer not to open the subclassing debate.
If you support subclassing, I will add a flag. My only objection is
that it is much easier to add this functionality in the future than to
remove it if it gets abused.  Can we postpone this decision until real
life use case comes in?  (Subclass to change dst() in itself is not a
use case.  Why do you need legacy timetuple interface to work is the
first question.)

> - 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)"?
>

Please make a call and I'll implement it.  The competing spellings are:

1. timedelta(hours=24)
2. 24 hours (I rejected this because it may be confused with an int)
3. timedelta(days=1)
4. in the range [-timedelta(hours=23, minutes=59), timedelta(hours=23,
minutes=59)]

Yes, there was a reason, but I don't think it is a good one.  The
default implementation of fromutc() does not work if utcoffset()
returns timedelta but dst() returns None.  The dst() value used to
detect DST ambiguities.  Remember, fromutc() despite the name is
defined to operate on local times and some local times are ambiguous
and some are invalid.  The default implementation attempts to detect
that.  This is the issue that I am trying to avoid in my design.

The test is easy to add.  My longer term plan is to disallow passing
argument to constant methods, so adding check is somewhat wasteful.
Will do.

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

Yes.  this was my transition plan.  Make it optional.  Modify datetime
methods to first try to get offset/name/dst without passing time
argument and fall back to passing self.  Remove optional argument from
timezone methods.

Yes.  I want parseable __repr__ as well.  I've been torn between
'datetime.timezone(datetime.timedelta(..)[, ..])'  and
'timezone(timedelta(..)[, ..])'. Opinions welcome.  Similarly, should
str(timezone.utc) be '+0000' or 'UTC' or '+00:00'?

You are reading my mind.  I planned to tear out conditional logic that
supports plain 'UTC', but apparently forgot.  Will do.

> - 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...

You are not alone! :-)  My understanding is that utcoffset was
designed around the notion that if you add a day to 12 noon the day
before DST change, you should still get 12 noon the next day.
Supporting this requires deriving local timezone from local time which
is impossible for some datetime values.  Most business applications
can ignore this because nobody schedules meetings between 1 and 2 am
and those who do are smart enough to catch an exception and ask the
user to clarify what time he/she means.

Good catch.  Thanks for the review.

I have to stop replying to emails.  There is no reason behind roundup remove ">" comments logic.   Reposting my message:

"""
On Fri, Jun 11, 2010 at 10:23 AM, Mark Dickinson <report@bugs.python.org> wrote:
..
> Some comments from playing with this patch (without having looked at the implementation):
>
thank you very much for your comments.  As we are fine-tuning the
timezone class, do you think it would be helpful to go back to Brett's
suggestion and add datetime.py with just from _datetime import * and
keep parallel Python and C implementations of timezone class only
until full datetime.py is available?

> - 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.
>
I was deliberate in a sense that I thought about it, but there was no
substantial discussion.  Original patch did not allow subclassing of
the UTC class and I thought it was right.  (See msg106415 and
msg106422.) subclassing FixedOffsetTimeZone makes more sense, but I
was hoping to keep timezone complete and eventually stop supporting
tzinfo API (not allow passing datetime in the methods.)  More on this
below.  I would very much prefer not to open the subclassing debate.
If you support subclassing, I will add a flag. My only objection is
that it is much easier to add this functionality in the future than to
remove it if it gets abused.  Can we postpone this decision until real
life use case comes in?  (Subclass to change dst() in itself is not a
use case.  Why do you need legacy timetuple interface to work is the
first question.)


> - 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)"?
>

Please make a call and I'll implement it.  The competing spellings are:

1. timedelta(hours=24)
2. 24 hours (I rejected this because it may be confused with an int)
3. timedelta(days=1)
4. in the range [-timedelta(hours=23, minutes=59), timedelta(hours=23,
minutes=59)]


> - 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?
>
Yes, there was a reason, but I don't think it is a good one.  The
default implementation of fromutc() does not work if utcoffset()
returns timedelta but dst() returns None.  The dst() value used to
detect DST ambiguities.  Remember, fromutc() despite the name is
defined to operate on local times and some local times are ambiguous
and some are invalid.  The default implementation attempts to detect
that.  This is the issue that I am trying to avoid in my design.


> - 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.
>
The test is easy to add.  My longer term plan is to disallow passing
argument to constant methods, so adding check is somewhat wasteful.
Will do.

> - 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?

Yes.  this was my transition plan.  Make it optional.  Modify datetime
methods to first try to get offset/name/dst without passing time
argument and fall back to passing self.  Remove optional argument from
timezone methods.

I can make the arg optional now.
>
> - Any chance of a nice __str__ implementation for timezone instances?
>  (And/or possibly a nice __repr__ as well)?
>
Yes.  I want parseable __repr__ as well.  I've been torn between
'datetime.timezone(datetime.timedelta(..)[, ..])'  and
'timezone(timedelta(..)[, ..])'. Opinions welcome.  Similarly, should
str(timezone.utc) be '+0000' or 'UTC' or '+00:00'?

> - 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.
>
You are reading my mind.  I planned to tear out conditional logic that
supports plain 'UTC', but apparently forgot.  Will do.

> - 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...

You are not alone! :-)  My understanding is that utcoffset was
designed around the notion that if you add a day to 12 noon the day
before DST change, you should still get 12 noon the next day.
Supporting this requires deriving local timezone from local time which
is impossible for some datetime values.  Most business applications
can ignore this because nobody schedules meetings between 1 and 2 am
and those who do are smart enough to catch an exception and ask the
user to clarify what time he/she means.

> - In the docs, replace "timezeone" with "timezone"
>
Good catch.  Thanks for the review.

I am attaching a new patch, issue5094e.diff which addresses most of Mark's comments.  I left out repr() because two opinions were voiced on IRC with respect to datetime. prefix.  I would like to give it some more thought even though I am leaning towards compatibility with existing reprs.

I did not make td argument optional and did not allow timezone o be subclassed because these seem to be mutually exclusive options.  (If td is optional in base class, it must be optional in subclasses per Liskov's principle severely limiting utility of subclasses.)  Let's address this separately.

Minor notes:

msg107186:
   1. The constructor now accepts only whole number of minutes in [-23:59, 23:59] range.

rfc 3339 provides the following example:

   1937-01-01T12:00:27.87+00:20

   This represents the same instant of time as noon, January 1, 1937,
   Netherlands time.  Standard time in the Netherlands was exactly 19
   minutes and 32.13 seconds ahead of UTC by law from 1909-05-01 through
   1937-06-30.  This time zone cannot be represented exactly using the
   HH:MM format, and this timestamp uses the closest representable UTC
   offset.

The presence of fractions of seconds in time zone is an exception so
it might not be worth to support it but it exists.


msg107552:
   Similarly, should str(timezone.utc) be '+0000' or 'UTC' or '+00:00'?

Excerpts in favor for '+00:00' from rfc 3339:

   Attempts to label local offsets with alphabetic
   strings have resulted in poor interoperability in the past [IMAIL],
   [HOST-REQ].  As a result, RFC2822 [IMAIL-UPDATE] has made numeric
   offsets mandatory.

   If the time in UTC is known, but the offset to local time is unknown,
   this can be represented with an offset of "-00:00".  This differs
   semantically from an offset of "Z" or "+00:00", which imply that UTC
   is the preferred reference point for the specified time.

There is a separate issue #5288 asking to support sub-minute offsets.  This is not hard, but the C code still has a few interfaces left from the time when offset was an integer # of minutes.  I am +1 to fix that, but not as a part of this issue.

On str(tz), I definitely want an invariant str(tz) == tz.tzname(None).  I am open to changes to tzname(), but we are very close to bikesheding here.  Let's settle for 'UTC±HH:MM'.  This seems to be the most common spelling for numeric timezones in various tables on the web.

I have made my mind on subclassing timezone issue.  I believe subclassing should not be allowed and here is the reason:

The new datetime.timezone class is a very specific implementation of tzinfo interface.  It guarantees that utcoffset(dt) and friends return the same value for all times.  Applications should be able to rely on this and a simple isinstance(tz, timezone) should be enough to deduce that tz is a fixed offset tzinfo instance.  (Of course careful applications can check type(tz) == timezone instead, but making isinstance(tz, timezone) subtly wrong is not a good idea.)

There is also very little to be gained from subclassing timezone.  It defines only 4 methods and each is entirely trivial.  Any non-trivial tzinfo implementation will need to override all 4 of them anyways.  It is much safer to derive from tzinfo directly and possible reuse timezone through containment (for example to format offsets in a standard way.)

The only immediate application of subclassing is to add dst() method returning a fixed offset rather than None.  This is trivial to implement through containment and if such class becomes popular, I would prefer to add this functionality to timezone class itself.

I agree it seems fine to disallow subclassing of timezone.  (And if we decide, in the light of new information, that that was wrong, then it's easy to allow subclassing later;  harder to disallow it.)

+1 for 'UTC±HH:MM' for both tzname and __str__, too.

I'll take a look at the newest patch.

> +1 for 'UTC±HH:MM' for both tzname and __str__, too.

It looks like I subconsciently allocated enough characters for this in the string buffer, so now this is a single character change.  I'll do it with other changes (if any) before commit.

Attaching issue5094f.diff which implements 'UTC±HH:MM' and adds unit tests for more error cases.

The latest patch looks good to me.  I only have minor comments, in random order:

- Should the PyDateTime_TimeZone struct definition go into datetime.h, so that it's avaiable if you want to export any C-API functions later on?

- If you're not allowing subclassing, then presumably you don't need
  the new_timezone / new_timezone_ex dance?

- For clarity, please consider adding parentheses in:

    self = (PyDateTime_TimeZone *)type->tp_alloc(type, 0);

  to make it:

    self = (PyDateTime_TimeZone *)(type->tp_alloc(type, 0));

- Whitespace issues: there are a couple of tabs in the source (at around lines 810, 3388, 3390), and an overly long line (>79 characters) at around line 3365.

- Please add a brief comment before the added C functions (like new_timezone_ex) explaining their purpose.

- I wonder whether __ne__ should return the correct result (rather than returning NotImplemented) for timezone instances.  I realize that it's not necessary to implement it in order for 'tz1 != tz2' to work, but it still makes me uncomfortable that it's not implemented.  OTOH, I agree with the decision not to allow timezone order comparisons (though I bet they get requested at some point).

- There doesn't seem to be any mention of timezone.min or timezone.max in the docs.  Is this deliberate?

On Sun, Jun 13, 2010 at 10:09 AM, Mark Dickinson <report@bugs.python.org> wrote:
..
> - Should the PyDateTime_TimeZone struct definition go into
> datetime.h, so that it's avaiable if you want to export any C-API
> functions later on?
>
The original patch had this in the header file, but I moved it down to .c during dst debate. (See msg107186, point 3.)  In the future we may add accessor functions to datetime.h, but this can be done without exposing the C struct.
 
> - If you're not allowing subclassing, then presumably you don't need
>  the new_timezone / new_timezone_ex dance?
>
I agree, but if you don't mind, I will not change it at this point.  There is a small chance that there will be an outcry for supporting subclassing and we will put that back in. Also, issue #2267 debate may result in removing the dance from other factory functions. (If the camp saying that the base class has no idea how to construct subclass instances wins.) Not a big deal either way, though.

> - For clarity, please consider adding parentheses in:
>
>    self = (PyDateTime_TimeZone *)type->tp_alloc(type, 0);
>

will do.  PEP 7 does not mention that, but probably should. 

> - Whitespace issues: there are a couple of tabs in the source (at
> around lines 810, 3388, 3390), and an overly long line (>79
> characters) at around line 3365.
>

Thanks.  I thought I checked those.  Maybe I should take over issue8912 as a community service. :-) 

> - Please add a brief comment before the added C functions (like
> new_timezone_ex) explaining their purpose.
>

Will do.

> - I wonder whether __ne__ should return the correct result (rather
> than returning NotImplemented) for timezone instances. 

Will do.

>  OTOH, I agree with the decision not to allow timezone order
> comparisons (though I bet they get requested at some point).

I will support such request.  It is very easy to implement (just remove the check that disallows it) and makes perfect sense.  I left them out only because it is easier to add features than to remove in the future and I don't want to debate this now.


>
> - There doesn't seem to be any mention of timezone.min or timezone.max
> in the docs.  Is this deliberate?
>
Yes.  Without ordering, having min and max is rather strange.  I wanted to expose these for testing and will definitely document them when (and if) order comparisons are implemented.  For now, let's keep them as easter eggs.

Just to add a little bit of historical perspective, this proposal is really more than seven years old:

"""
s.keim (Dec 20, 2002 3:33 am; Comment #13)

.. do we really need methods like utcnow. I believe the following could be a little more easy to learn:

* the module define an UTC tzinfo object

* the module define an UTC tzinfo object instead of utcfromtimestamp(x) and utcnow(), the user can use fromtimestamp(x, tzinfo=UTC) and now(tzinfo=UTC)
""" http://www.zope.org/Members/fdrake/DateTimeWiki/SuggestedRequirements

Apparently, this proposal have been forgotten and reinvented again in msg106411, point 2.

issue5094g.diff addresses all Mark's suggestions except making struct definition public.  I also made a few other changes:

1. Constructor now raises TypeError when offset is not a timedelta instead of ValueError in previous version.

2. NEWS entry no longer says that argument is ignored by tzinfo methods implemented in timezone.  (Since these methods now check the type of the argument, it would be misleading to say that arg is ignored.)

3. Added some more tests for error cases.

4. Added ACK for Rafe Kaplan as the author of the original patch.

Also I changed Py_DECREFs in destructor to Py_CLEAR.  I understand that while not strictly required in this case, it is a good practice.

issue5904g.diff looks good to me.  A very nice piece of work!

Committed in r81981.  I'll open a separate documentation issue to update Doc/includes/tzinfo-examples.py and improve TZ related documentation.

Thanks for sticking with this, Alexander. I realized I was a slight pain to deal with on this one. =)

Reopening to add some minor fixes to tests and documentation.  See issue5094h.diff.  Ezio, thanks for finding these issues.

+1 for replacing math range notation with English. Much easier for non-math people that do Python :)

One more nit: Your docstrings use verb forms like “Returns” where PEP 257 advises to use “Return”: “[the docstring] prescribes the function or method's effect as a command ("Do this", "Return that"), not as a description; e.g. don't write "Returns the pathname ..."”

What about 

"""
.. method:: datetime.utcoffset()

   If :attr:`tzinfo` is ``None``, returns ``None``, else returns ...
"""

Should this use "return" too?

.. method:: datetime.utcoffset()

   Return ``None`` if :attr:`tzinfo` is ``None``, else else return ...

That said, to keep diffs readable, perhaps stick with the existing
convention know, and later a PEP 257 compliance diff can be made.

How to specify offset range horse got its beating above.  See msg107554.  The current wording is the best compromise between verbosity and precision.

Replacing the patch with one that fixes other nits.

Suggestion for one line:
“The *dt* argument must be aware with ``tzinfo`` set to ``self``.”
“The *dt* argument must be an aware datetime, with ``tzinfo`` set to ``self``.”

If there is a reST construct for aware datetime, use it. Since “self” is not special, perhaps an alternate wording like “the timezone instance from which the method is called”, but less ugly.

Some PyDoc_STR calls still use “Returns.”

By the way, you could have committed the unittest changes directly.

Good job. Thanks for working on this.

It is possible to backport this to future of Python 2.7?

> It is possible to backport this to future of Python 2.7?

No.

On this happy note, I am closing this issue.  Doc changes have been committed in r82004, test changes in r82003.  For repr(timezone(..)) development, please follow issue #9000.