datetime lacks concrete tzinfo implementation for UTC #49344
Comments
|
When you call datetime.datetime.utcnow() you get back a naive datetime It would be nice to have a concrete UTC tzinfo class that utcnow() uses If people have no issues with making this happen I will write the code |
|
Brett, 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 <[email protected]> wrote:
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 |
|
The UTC class have to be converted to C. Can someone write a patch for |
|
On Tue, Feb 3, 2009 at 03:28, STINNER Victor <[email protected]> wrote:
Yes, the example code is just an example. =)
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 |
|
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:
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 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 <[email protected]> wrote:
Well, I respectfully disagree. This advise seems to be placing Note that I am not suggesting passing anything to utcnow(). I would |
I am not sure what you mean by "muck with." Why would anyone want to subclass UTC?
There are several advantages of having all datetime instances with tzinfo=UTC() referring to the same instance:
>>> 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.
|
The latter would break compatibility, though (especially given how I also agree with Brett that a singleton looks rather unnecessary (it On the subject of the patch:
|
|
On Tue, May 25, 2010 at 5:45 AM, Antoine Pitrou <[email protected]> wrote:
I still don't understand your aversion to singletons and you did not We can reach a middle ground by interning UTC instances behind the |
|
On Mon, May 24, 2010 at 11:06 PM, Rafe Kaplan <[email protected]> wrote:
I actually like your original idea. It seems wasteful to create a |
|
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 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. |
|
.. Well, the datetime module is not exactly the place you want to start
This is exactly my preferred solution. |
|
"Note that I am not suggesting passing anything to utcnow(). I would 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: -----
Well, the datetime module is not exactly the place you want to start
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. |
|
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:
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, The presence of fractions of seconds in time zone is an exception so msg107552: Excerpts in favor for '+00:00' from rfc 3339: Attempts to label local offsets with alphabetic If the time in UTC is known, but the offset to local time is unknown, |
|
There is a separate issue bpo-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. |
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:
self = (PyDateTime_TimeZone *)type->tp_alloc(type, 0);to make it: self = (PyDateTime_TimeZone *)(type->tp_alloc(type, 0));
|
|
On Sun, Jun 13, 2010 at 10:09 AM, Mark Dickinson <[email protected]> wrote:
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.
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 bpo-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.
will do. PEP-7 does not mention that, but probably should.
Thanks. I thought I checked those. Maybe I should take over bpo-8912 as a community service. :-)
Will do.
Will do.
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.
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: """ .. do we really need methods like utcnow. I believe the following could be a little more easy to learn:
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:
|
|
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. |
abalkin
changed the title
|
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 """ 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 |
|
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: 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? |
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 bpo-9000. |
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:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: