-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
tz_compare fails to consider string "UTC" and pytz UTC object equal #23959
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
This slipped through the cracks when I did some UTC refactoring. We don't rely on |
QQ: Would we expect |
I'm not sure, but it probably depends on exactly how we're using The docs say we compare "string representations", and they're clearly different In [10]: str(dateutil.tz.UTC)
Out[10]: 'tzutc()'
In [11]: str(pytz.timezone("UTC"))
Out[11]: 'UTC' cc @pganssle if you have thoughts. |
Sorry, I have somewhat let this fall by the wayside. Unfortunately, I don't know of any reasonably generic way to compare time zones, and the semantics of timezone aware datetime comparison unfortunately use The reason for NYC = pytz.timezone('America/New_York')
dt1 = NYC.localize(datetime(2018, 1, 1))
dt2 = NYC.localize(datetime(2018, 6, 1))
print(dt1.tzinfo is dt2.tzinfo)
# False I am not aware of whether In any case, you may be able to special-case some logic related to UTC, but unfortunately the semantics are already messed up and you may in fact be creating more weird edge cases and inconsistencies by trying to patch in compatibility between time zone providers, or to try to support string-to-timezone comparisons directly. Hopefully we will be making the |
By the way, my solution to this that should unravel most of the complicated stuff you are doing around time zones is to create your own IANA time zone implementation. The two representations you are relying on are not designed for your use case, and as a result you make ample use of private methods, which is a much higher maintenance burden than just creating your own time zone implementation. I think you should somewhat reliably be able to create a mapping between input time zones and the IANA zones they map to, so the extent of your support for third party time zones would be to immediately convert them to an internal pandas TZ representation. This would allow you to drop |
what would this look like? |
Taking some time to sketch out some sort of prototype is on my list, but unfortunately it's not high on my list. I think the high level overview is:
For strings, the support is built in already. For There will be time zones that cannot be translated in this way, but I don't see that this is a big deal, because this is "fast path" time zone handling code. You can have two levels of support. UTC, fixed offset time zones and IANA zones represented as strings, If done right, this doesn't actually require a dependency on Most of this does not have to be a breaking change, though dropping |
This is partially fixed as tz_compare now requires tzinfo objects, so the string "UTC" is irrelevant. It's still questionable that |
questionable? it just wasted yet another day in my life. |
Nothing user-facing here, but this surprised me.
Other timezones behave as expected
Is this user error, or a bug?
The text was updated successfully, but these errors were encountered: