python / cpython

The Python programming language
https://www.python.org
Other
63.33k stars 30.32k forks source link

Comparing datetime.time objects incorrect for TZ aware and unaware #82993

Open 016ff70b-1bbb-4b35-a371-2b7d6d7c1b0b opened 4 years ago

016ff70b-1bbb-4b35-a371-2b7d6d7c1b0b commented 4 years ago
BPO 38812
Nosy @jsnklln, @pganssle, @mikessut

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: ```python assignee = None closed_at = None created_at = labels = ['library', '3.9'] title = 'Comparing datetime.time objects incorrect for TZ aware and unaware' updated_at = user = 'https://github.com/mikessut' ``` bugs.python.org fields: ```python activity = actor = 'Jason.Killen' assignee = 'none' closed = False closed_date = None closer = None components = ['Library (Lib)'] creation = creator = 'epicadv' dependencies = [] files = [] hgrepos = [] issue_num = 38812 keywords = [] message_count = 5.0 messages = ['356673', '356706', '356806', '356814', '356881'] nosy_count = 3.0 nosy_names = ['Jason.Killen', 'p-ganssle', 'epicadv'] pr_nums = [] priority = 'normal' resolution = None stage = None status = 'open' superseder = None type = None url = 'https://bugs.python.org/issue38812' versions = ['Python 3.9'] ```

016ff70b-1bbb-4b35-a371-2b7d6d7c1b0b commented 4 years ago
import pytz
import datetime

tzaware_time1 = datetime.time(7,30,tzinfo=pytz.timezone("America/Denver"))
tzaware_time2 = datetime.time(7,30,tzinfo=pytz.utc)

tzunaware_time = datetime.time(7, 30)

# This fails with exception: TypeError: can't compare offset-naive and offset-aware times # even though both ARE tz aware. tzaware_time1 \< tzaware_time2

# This does NOT raise an exception and should since one is aware and one isn't. tzunaware_time \< tzaware_time1

3659106c-decb-49f8-81c7-40939c950c65 commented 4 years ago

This appears to be a bug in pytz which as far as I can tell is not part of the "standard" lib, at least it's not in Lib.

Running this example which does not use pytz: from datetime import timedelta, datetime, tzinfo, timezone, time

# stole this from the docs, and beat it up a bit
class KabulTz(tzinfo):
    def utcoffset(self, dt):
        return timedelta(hours=4, minutes=30)

    def fromutc(self, dt):
        # Follow same validations as in datetime.tzinfo
        if not isinstance(dt, datetime):
            raise TypeError("fromutc() requires a datetime argument")
        if dt.tzinfo is not self:
            raise ValueError("dt.tzinfo is not self")
    return dt + timedelta(hours=4)
    def dst(self, dt):
        # Kabul does not observe daylight saving time.
        return timedelta(0)

    def tzname(self, dt):
        return "+04"

tzaware_time2 = time(7,30,tzinfo=timezone.utc)
tzaware_time1 = time(7,30,tzinfo=KabulTz())

tzunaware_time = time(7, 30)

tzaware_time1 \< tzaware_time2

I get no error.

When I use your example I notice that utcoffset in pytz/tzinfo.py returns None for the Denver example but for the UTC object returns ZERO which happens to be timedelta(0) which seems correct. It's weird to me that pytz is returning None in the Denver case because I think what it should return is right there in self._utcoffset.

More info: It looks like pytz isn't handling the fact that it's a time only type. dt appears to be a datetime when utcoffset is called from a datetime.

016ff70b-1bbb-4b35-a371-2b7d6d7c1b0b commented 4 years ago

Ok. I'll file a bug on pytz. Thanks!

On Sat, Nov 16, 2019 at 11:18 PM Karthikeyan Singaravelan \< report@bugs.python.org> wrote:

Change by Karthikeyan Singaravelan \tir.karthi@gmail.com\:

---------- nosy: +p-ganssle


Python tracker \report@bugs.python.org\ \https://bugs.python.org/issue38812\


pganssle commented 4 years ago

I do not think this is a bug in pytz, but if it's a bug in Python it's one in reporting what the error is.

The issue is that the time zone offset for "rules-based zones" like America/Denver (i.e. most time zones) is *undefined for bare times, because the offset that apply depends on the *date and the *time*.

The documentation for tzinfo.utcoffset specifies that if the offset is unknown, a time zone offset should return None: https://docs.python.org/3/library/datetime.html#datetime.tzinfo.utcoffset

The documentation for determining whether an object is aware or naive also specifies that if utcoffset() returns None, the object is naive (even if tzinfo is not None): https://docs.python.org/3/library/datetime.html#determining-if-an-object-is-aware-or-naive

So basically, everyone is doing the right thing except the person who attached this pytz time zone to a time object (as a side note, it may be worth reading this blog post that explains why the way this time zone is attached to the time object is incorrect: https://blog.ganssle.io/articles/2018/03/pytz-fastest-footgun.html).

That said, we may be able to improve the error message raised here by distinguishing between the case where there's no tzinfo at all and the case where utcoffset() returns None. I think we can change the exception message to have a helpful hint like, "cannot compare offset-naive and offset-aware times; one of the operands is offset-naive because its offset is undefined."

We could possibly be even more specific.

3659106c-decb-49f8-81c7-40939c950c65 commented 4 years ago

Yep I wasn't seeing the forest for the trees. Thanks for clearing that up. I'll swoop back in and see what I can do.