Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Properly handle UTC conversion in date2num. #6262
Conversation
mdboom
added the
needs_review
label
Apr 2, 2016
pganssle
commented on an outdated diff
Apr 2, 2016
| @@ -20,6 +22,8 @@ | ||
| import matplotlib.pyplot as plt | ||
| import matplotlib.dates as mdates | ||
| +from numpy import array |
pganssle
Contributor
|
|
It may be worth adding a test that makes sure this works during a DST->STD transition, where there is a fold, but I suspect that in general that would mostly be a test of the time zone object's ability to handle ambiguous dates (see the extensive discussion at dateutil/dateutil#225 and the various linked discussions for more information than you could ever want about this), so I don't see any significant need for that. The best advice for people is that if they want to use |
tacaswell
added this to the
1.5.2 (Critical bug fix release)
milestone
Apr 2, 2016
|
I am glad someone who is not me understands dates We do have some pandas-optional tests floating around already (I think mostly in test_axes) and install pandas on both travis and appveyor so if you want to use pandas you can. |
|
OK, I can add a pandas-specific test. Any idea why the Appveyor build is failing? I got that same failure locally on linux, but since I didn't touch anything related to that test, I figured it was just some weird version-specific problem. |
|
The appveyor is ses is likely due too #5950 |
pganssle
and 1 other
commented on an outdated diff
Apr 2, 2016
| - if td_remainder > 0: | ||
| - base += td_remainder / SEC_PER_DAY | ||
| + # Append the seconds as a fraction of a day | ||
| + base += _total_seconds(dt - rdt) / SEC_PER_DAY |
pganssle
Contributor
|
|
Do y'all want me to rebase this so it passes the appveyor tests, or should I just leave it as is? |
efiring
commented on the diff
May 2, 2016
| @@ -212,47 +212,31 @@ def _to_ordinalf(dt): | ||
| days, preserving hours, minutes, seconds and microseconds. Return value | ||
| is a :func:`float`. | ||
| """ | ||
| - | ||
| - if hasattr(dt, 'tzinfo') and dt.tzinfo is not None: | ||
| - delta = dt.tzinfo.utcoffset(dt) | ||
| - if delta is not None: | ||
| - dt -= delta | ||
| + # Convert to UTC | ||
| + tzi = getattr(dt, 'tzinfo', None) | ||
| + if tzi is not None: | ||
| + dt = dt.astimezone(UTC) |
pganssle
Contributor
|
efiring
commented on the diff
May 16, 2016
| base = float(dt.toordinal()) | ||
| - if isinstance(dt, datetime.datetime): | ||
| - # Get a datetime object at midnight in the same time zone as dt. | ||
| - cdate = dt.date() | ||
| - midnight_time = datetime.time(0, 0, 0, tzinfo=dt.tzinfo) | ||
| + | ||
| + # If it's sufficiently datetime-like, it will have a `date()` method | ||
| + cdate = getattr(dt, 'date', lambda: None)() | ||
| + if cdate is not None: |
efiring
Owner
|
mdboom
modified the milestone: 2.0.1 (next bug fix release), 1.5.2 (Critical bug fix release)
May 16, 2016
|
Pinging - any update on this PR? |
tacaswell
closed this
Jul 25, 2016
tacaswell
reopened this
Jul 25, 2016
tacaswell
added needs_review and removed needs_review
labels
Jul 25, 2016
tacaswell
merged commit 0a31b36
into matplotlib:master
Jul 26, 2016
tacaswell
removed the
needs_review
label
Jul 26, 2016
tacaswell
added a commit
that referenced
this pull request
Jul 26, 2016
|
|
tacaswell |
64756ee
|
|
backported to v2.x as 64756ee |
pganssle commentedApr 2, 2016
Fixes #3896.
I believe this is a much better way to do it than the current method where we're trying to pull the
utcoffset. The issue in #3896 actually has more to do with the waypytzhandles normalization and the way thatpandasstoresTimeStampobjects. As you can see from the test I added, a normal list ofdatetimeobjects withpytztime zones won't trigger the bug, but rather than pulling inpandasjust for the test, I mocked out a datetime object that basically does the same thing pandas is doing.