Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pd.Timedelta.resolution is different from datetime.timedelta.resolution #21344

Closed
mroeschke opened this issue Jun 6, 2018 · 8 comments

Comments

@mroeschke
Copy link
Member

commented Jun 6, 2018

timedelta.resolution is documented as:

The smallest possible difference between non-equal timedelta objects, timedelta(microseconds=1).

while pandas.Timedelta.resolution is doc-string'd as:

return a string representing the lowest resolution that we have

In [9]: pd.Timedelta(minutes=30).resolution
Out[9]: 'T'

In [10]: timedelta(minutes=30).resolution
Out[10]: datetime.timedelta(0, 0, 1)

I think this difference should be addressed by either:

  1. Documenting the difference between both but also supporting datetime.timedelta's notion of resolution in Timedelta (i.e. something that returns Timedelta(nanoseconds=1))
  2. Making Timedelta.resolution consistent with timedelta.resolution and creating a new property for "the lowest resolution" of a Timedelta instance

Thoughts?

xref #21336

@gfyoung

This comment has been minimized.

Copy link
Member

commented Jun 6, 2018

I would prefer option 1. Option 2 would commit us (to some extent) to tracking datetime, which is not necessarily bad, but I do prefer flexibility in general.

@drewmassey

This comment has been minimized.

Copy link

commented Jun 7, 2018

(Sorry this was initially posted against the wrong issue):
I'm looking at

def resolution(self):
and I think fixing this would be breaking change ... it isn't hard to change the return values but is that really the desired behavior?

@mroeschke

This comment has been minimized.

Copy link
Member Author

commented Jun 7, 2018

@drewmassey if we move forward with option 1. Then we would:

  1. Make a note in pd.Timedelta.resolution that this is different from datetime.timedelta
  2. Create a separate property for pd.Timedelta to return pd.Timedelta(nanoseconds=1)

If we move forward with option 2 then yes it would be a breaking change. I would slightly prefer option 1 as well since it doesn't disrupt the current API.

@drewmassey

This comment has been minimized.

Copy link

commented Jun 7, 2018

Ok I can handle this. Let me get the PR through for issue #21336 to make sure I have the pandas workflow in shape and then I'll submit this one.

@jreback jreback added this to the 0.24.0 milestone Jun 13, 2018

@jreback jreback modified the milestones: 0.24.0, 0.25.0 Oct 23, 2018

@jreback jreback modified the milestones: 0.25.0, 1.0 Apr 20, 2019

@jbrockmendel

This comment has been minimized.

Copy link
Member

commented Jun 11, 2019

I think we should deprecate the current behavior and eventually change it to match the stdlib. @mroeschke thoughts?

@mroeschke

This comment has been minimized.

Copy link
Member Author

commented Jun 11, 2019

Agreed that we should match the stdlib.

@jbrockmendel

This comment has been minimized.

Copy link
Member

commented Jun 13, 2019

Working on this I'm noticing that we also have Timestamp.resolution subtly different from datetime.resolution. On datetime it is a class attribute, whereas on Timestamp it is a property

@jbrockmendel

This comment has been minimized.

Copy link
Member

commented Jun 13, 2019

Yikes, and pd.NaT.resolution has the datetime behavior

@jbrockmendel jbrockmendel referenced this issue Jun 13, 2019

Merged

DEPR: deprecate Timedelta.resolution #26839

4 of 4 tasks complete

@jreback jreback modified the milestones: 1.0, 0.25.0 Jun 14, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.