Skip to content

Timestamp.resolution returns timedelta(microseconds=1) #21336

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

Closed
jbrockmendel opened this issue Jun 6, 2018 · 7 comments · Fixed by #21861
Closed

Timestamp.resolution returns timedelta(microseconds=1) #21336

jbrockmendel opened this issue Jun 6, 2018 · 7 comments · Fixed by #21861
Milestone

Comments

@jbrockmendel
Copy link
Member

I hadn't even noticed that resolution was a datetime property until now, but it returns timedelta(0, 0, 1). Timestamp doesn't override that. I expect the correct thing to do is to return Timedelta(nanoseconds=1). Pretty low priority.

@gfyoung gfyoung added API Design Timedelta Timedelta data type labels Jun 6, 2018
@gfyoung
Copy link
Member

gfyoung commented Jun 6, 2018

Pretty low priority.

@jbrockmendel : How easy would it be to patch?

@jbrockmendel
Copy link
Member Author

super-duper easy. Might merit a "Good First Issue" tag.

@gfyoung
Copy link
Member

gfyoung commented Jun 6, 2018

Might merit a "Good First Issue" tag.

Then let's label as such. Done.

@uds5501
Copy link
Contributor

uds5501 commented Jun 7, 2018

@jbrockmendel @gfyoung Any guidance for the patch of the same?

@jbrockmendel
Copy link
Member Author

You'll want to define a property on the Timestamp class that just returns Timedelta(nanoseconds=1). For the sake of thoroughness you'll need a super-simple test in pandas.tests.scalar.timestamp.test_timestamp.

@drewmassey
Copy link

drewmassey commented Jun 7, 2018

So 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
Copy link
Member

mroeschke commented Jun 7, 2018

@drewmassey your snippet is looking at timedeltas.pyx (which has a relevant resolution issue open #21344). This issue would entail adding a resolution property to timestamp.pyx

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants