Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed

Conversation

dstansby
Copy link
Member

This allows_to_ordinalf to handle pythontimedelta objects. 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, whereasdelta being atimedelta object was a problem before:

importmatplotlib.pyplotaspltimportmatplotlib.patchesasmpatchfromdatetimeimportdatetime,timedeltastart=datetime(2017,1,1,0,0,0)delta=timedelta(seconds=16)patch=mpatch.Rectangle((start,0),delta,1)fig,ax=plt.subplots()ax.add_patch(patch)ax.set_xlim(start-delta,start+2*delta)ax.set_ylim(-1,2)plt.show()

@tacaswell
Copy link
Member

Can you add a test for this?

@tacaswelltacaswell added this to the2.1 (next point release) milestoneJun 9, 2017
@dstansbydstansby mentioned this pull requestJul 12, 2017
6 tasks
@dstansbydstansby mentioned this pull requestJul 24, 2017
@tacaswell
Copy link
Member

@dstansby This needs a rebase.

"""
# 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 atimedelta. 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 totimedelta.

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

@pgansslepganssleAug 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 inthis comment.timedelta is a weird animal in that you normally don't have separate units forx anddelta-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)datetimecould be used as a width inrect, which is just as wrong astimedelta failing to work (and this doesnot 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 ofx ordelta-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 thatideally the way this would work is thatheight andwidth would apply to the values in the initial units / types, if possible.

Consider that if the plot's width is calculated by addingwidth todt_begin, you automatically get the ability to adddateutil.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.

dstansby reacted with thumbs up emoji
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.

@pganssle
Copy link
Member

Is this supplanted by#9072?

@dstansby
Copy link
MemberAuthor

dstansby commentedAug 25, 2017 via email

Yes and no... If one wants to plot timedelta objects by themselvespresumably there needs to be a converter, but maybe it's best to put itinto 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>.

@pganssle
Copy link
Member

I think it should definitely be its own thing andnot folded intodates like this. It's somewhat meaningful to wrapdate,time anddatetimetogether, buttimedelta` is another beast entirely.

@tacaswelltacaswell modified the milestones:2.2 (next next feature release),2.1 (next point release)Aug 26, 2017
@tacaswell
Copy link
Member

Moving this to 2.2 due to@pganssle 's concerns about the implementation.

dstansby reacted with thumbs up emoji

@dstansby
Copy link
MemberAuthor

Was easier to just open a new PR at#9120

@dstansbydstansby deleted the timedelta-ordinal branchNovember 22, 2017 14:01
@QuLogicQuLogic modified the milestones:needs sorting,v2.2.0Feb 13, 2018
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@WeatherGodWeatherGodWeatherGod left review comments

@pgansslepgansslepganssle left review comments

@tacaswelltacaswelltacaswell approved these changes

Assignees
No one assigned
Projects
None yet
Milestone
v2.2.0
Development

Successfully merging this pull request may close these issues.

5 participants
@dstansby@tacaswell@pganssle@WeatherGod@QuLogic

[8]ページ先頭

©2009-2025 Movatter.jp