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

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

Closed
jbrockmendel opened this issue Jun 6, 2018 · 7 comments

Comments

Projects
None yet
6 participants
@jbrockmendel
Copy link
Member

commented Jun 6, 2018

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

This comment has been minimized.

Copy link
Member

commented Jun 6, 2018

Pretty low priority.

@jbrockmendel : How easy would it be to patch?

@jbrockmendel

This comment has been minimized.

Copy link
Member Author

commented Jun 6, 2018

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

@gfyoung

This comment has been minimized.

Copy link
Member

commented Jun 6, 2018

Might merit a "Good First Issue" tag.

Then let's label as such. Done.

@uds5501

This comment has been minimized.

Copy link
Contributor

commented Jun 7, 2018

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

@jbrockmendel

This comment has been minimized.

Copy link
Member Author

commented Jun 7, 2018

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

This comment has been minimized.

Copy link

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

This comment has been minimized.

Copy link
Member

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

jbrockmendel added a commit to jbrockmendel/pandas that referenced this issue Jul 11, 2018

@jbrockmendel jbrockmendel referenced this issue Jul 11, 2018

Merged

CLN: miscellaneous cleanups / fixes #21861

3 of 3 tasks complete

@jreback jreback added this to the 0.24.0 milestone Jul 12, 2018

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