Skip to content

DOC: timezone warning for dates beyond TODAY #34100

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Aug 21, 2020
Merged

DOC: timezone warning for dates beyond TODAY #34100

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
28b8a5b
DOC: timezone warning for dates beyond TODAY
joooeey May 10, 2020
2b584ab
shorter warning text
joooeey May 22, 2020
a37491b
Merge remote-tracking branch 'upstream/master' into patch-1
joooeey Jul 12, 2020
f2b17b0
Merge remote-tracking branch 'upstream/master' into patch-1
MarcoGorelli Aug 21, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions doc/source/user_guide/timeseries.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2314,13 +2314,24 @@ you can use the ``tz_convert`` method.
Instead, the datetime needs to be localized using the ``localize`` method
on the ``pytz`` time zone object.

.. warning::

Be aware that for times in the future, correct conversion between time zones
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think instead of adding a new warning box, the last sentence of the warning box below should be Be aware that for times in the future, correct conversion between time zones (and UTC) cannot be guaranteed by any time zone library

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel strongly that this should be a separate warning. These are two different issues (even though the solution is basically the same), and one sentence could easily be overlooked in the big 2038 warning. Here's my rationale as I said int he discussion at #33863 :

Okay. I'm convinced now. We should warn about converting (any) future time. I still think this warning shouldn't be an afterthought in the 2038 warning text. It should be its own warning box. That gives it the prominence it deserves and ensures the warning remains if the 2038 problem is resolved in the downstream libraries and that box is removed.

In addition, the example given in the warning below only concerns the 2038 problem. How is anyone reading the docs supposed to know that the example only concerns the first part of the warning (the 2038 problem) but not the last sentence. It's just confusing that way.

What do you think about adding the warning I proposed but only with the first sentence?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay maybe you can combine this warning and the 2038 warning into 1 warning, highlighting your point first and the 2038 issue as a ramification.

Just want to reduce the amount of warnings in this section since there's already a lot of them.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is a longer warning better than two warnings? Of the five warnings here, the first and second warnings about differing timezone definitions between libraries and library versions respectively are also not merged into one. For good reason, I would say. Keeping five separate warnings makes the structure quickest to comprehend and hence the user guide easiest to read, which I think is the ultimate goal. That's why there should be a separate, succinct warning about time zone conversions for future dates.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would agree this can at least be shortened a bit. We don't need to single out Morocco

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just committed a change to make it shorter but CI / Web and Docs throws: AttributeError: module 'numpy.random' has no attribute 'BitGenerator'. I have no idea what that means.

(and UTC) cannot be guaranteed by any time zone library. Sometimes the rules
governing a timezone's offset from UTC are changed. Authorities usually
announce such changes many months in advance but there have been examples of
much shorter lead times such as when Morocco announced just two days before
the planned switch from summer time to winter time in 2018 that the country
would stay on summer time permanently. Furthermore, the databases that
Pandas relies on may need some time to record planned changes to timezone
offsets.

.. warning::

If you are using dates beyond 2038-01-18, due to current deficiencies
in the underlying libraries caused by the year 2038 problem, daylight saving time (DST) adjustments
to timezone aware dates will not be applied. If and when the underlying libraries are fixed,
the DST transitions will be applied. It should be noted though, that time zone data for far future time zones
are likely to be inaccurate, as they are simple extrapolations of the current set of (regularly revised) rules.
the DST transitions will be applied.

For example, for two dates that are in British Summer Time (and so would normally be GMT+1), both the following asserts evaluate as true:

Expand Down