Skip to content

Allow timedelta to be converted to a ordinalf #8730

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

Closed
wants to merge 2 commits into from
Closed

Allow timedelta to be converted to a ordinalf #8730

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f64e676
Allow timedelta to be converted to a ordinalf
dstansby Jun 7, 2017
2fafd12
Add test for timedelta --> ordinalf conversion
dstansby Jun 8, 2017
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
9 changes: 6 additions & 3 deletions lib/matplotlib/dates.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,16 +207,19 @@ def _get_rc_timezone():

def _to_ordinalf(dt):
"""
Convert :mod:`datetime` or :mod:`date` to the Gregorian date as UTC float
days, preserving hours, minutes, seconds and microseconds. Return value
is a :func:`float`.
Convert :mod:`datetime`, :mod:`date` or :mod:`timedelta` to the Gregorian
date as UTC float days, preserving hours, minutes, seconds and
microseconds. Return value is a :func:`float`.
"""
# Convert to UTC
tzi = getattr(dt, 'tzinfo', None)
if tzi is not None:
dt = dt.astimezone(UTC)
tzi = UTC

if isinstance(dt, datetime.timedelta):
Copy link
Member

Choose a reason for hiding this comment

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

This seems like the wrong place to decide whether or not this is a timedelta. Despite both being about time, I think durations and points in time are very different things and should follow different code paths. Most of the date-handling stuff is simply not applicable to timedelta.

Copy link
Member

Choose a reason for hiding this comment

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

how else would you compute widths? It is a similar problem to temperatures. You can't simply convert a 5 degrees C temperature change into Fahrenheit temperature change (a big annoyance of mine when reading news articles that blindly injects unit conversions)

Copy link
Member

@pganssle pganssle Aug 21, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

@WeatherGod I expressed my reservations a bit more in this comment. timedelta is a weird animal in that you normally don't have separate units for x and delta-x, but in datetimes you do.

One problem with this as a solution to #4916 is that a corollary to #4916 is that (if I understand correctly) datetime could be used as a width in rect, which is just as wrong as timedelta failing to work (and this does not fix that problem).

I don't know enough about the way the width processing and unit framework to know what the right seam is, but I would think that the right place to do the type check would be earlier in the pipeline, when you still know whether this is supposed to be units of x or delta-x.

Copy link
Member

Choose a reason for hiding this comment

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

Another thing to note about this solution is that ideally the way this would work is that height and width would apply to the values in the initial units / types, if possible.

Consider that if the plot's width is calculated by adding width to dt_begin, you automatically get the ability to add dateutil.relativedelta.relativedelta objects as well (for example, relativedelta(weekday=MO(+3)) - the width being "from the initial value to three Mondays later). It's not high priority, but if you're solving this problem anyway, it's worth considering that leveraging the existing arithmetic framework for rich objects might be worth doing.

Copy link
Member

Choose a reason for hiding this comment

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

ooh, now that is an attractive option.

return dt.total_seconds() / SEC_PER_DAY

base = float(dt.toordinal())

# If it's sufficiently datetime-like, it will have a `date()` method
Expand Down
7 changes: 7 additions & 0 deletions lib/matplotlib/tests/test_dates.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -466,3 +466,10 @@ def test_tz_utc():
def test_num2timedelta(x, tdelta):
dt = mdates.num2timedelta(x)
assert dt == tdelta


def test_timedelta_ordinalf():
# Check that timedeltas can be converted to ordinalfs
dt = datetime.timedelta(seconds=60)
ordinalf = mdates._to_ordinalf(dt)
assert ordinalf == 1 / (24 * 60)