Allow timedelta to be converted to a ordinalf#8730
Allow timedelta to be converted to a ordinalf#8730dstansby wants to merge 2 commits intomatplotlib:masterfrom
Conversation
|
Can you add a test for this? |
|
@dstansby This needs a rebase. |
2808618 to
2fafd12
Compare
| dt = dt.astimezone(UTC) | ||
| tzi = UTC | ||
|
|
||
| if isinstance(dt, datetime.timedelta): |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
@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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
ooh, now that is an attractive option.
|
Is this supplanted by #9072? |
|
Yes and no... If one wants to plot timedelta objects by themselves
presumably there needs to be a converter, but maybe it's best to put it
into its own function.
On 25 Aug 2017 8:39 p.m., "Paul Ganssle" <notifications@github.com> wrote:
Is this supplanted by #9072
<#9072>?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#8730 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AF6RfGrDAvjCuPNYsi82Aq6ueM2pl4SFks5sbyL8gaJpZM4NzAte>
.
|
|
I think it should definitely be its own thing and not folded into |
|
Moving this to 2.2 due to @pganssle 's concerns about the implementation. |
|
Was easier to just open a new PR at #9120 |
This allows
_to_ordinalfto handle pythontimedeltaobjects. This is a first step in solving #4916, where the problem is that it is currently not possible to convert a width into a ordinal.e.g. the following now works with this PR, whereas
deltabeing atimedeltaobject was a problem before: