Skip to content

BUG: Bug in Timestamp-Timestamp not returning a Timedelta type (GH8865) #8981

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Dec 4, 2014
Merged

BUG: Bug in Timestamp-Timestamp not returning a Timedelta type (GH8865) #8981

merged 1 commit into from
Dec 4, 2014

Conversation

@jreback
Copy link
Contributor

@jreback jreback commented Dec 3, 2014

closes #8865
@jreback jreback added Bug Dtype Conversions Unexpected or buggy dtype conversions Datetime Datetime data dtype Timedelta Timedelta data type labels Dec 3, 2014
@jreback jreback added this to the 0.15.2 milestone Dec 3, 2014
@jreback
Copy link
Contributor Author

jreback commented Dec 3, 2014

cc @shoyer

was a rabbit-hole!

not 100% convinced that the auto-broadcasting strategy will work here, but go for it.
shoyer
shoyer reviewed Dec 3, 2014
View reviewed changes
pandas/tslib.pyx
Copy link
Member

@shoyer shoyer Dec 3, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure timezone comparison is a ValueError... it would be good to be consistent (IMO TypeError is the right choice)
Copy link
Contributor Author

@jreback jreback Dec 4, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

(Pdb) p dt_tz datetime.datetime(2013, 1, 1, 0, 0, tzinfo=<DstTzInfo 'US/Eastern' EST-1 day, 19:00:00 STD>) (Pdb) p dt datetime.datetime(2013, 1, 1, 0, 0) (Pdb) p dt_tz - dt *** TypeError: TypeError("can't subtract offset-naive and offset-aware datetimes",)

datetime-datetime has this as a TypeError (that's why I changed it, originally I had it as a ValueError as well)
Copy link
Member

@shoyer shoyer Dec 4, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, so maybe you should also change this for comparisons on line 813 above?
Copy link
Contributor Author

@jreback jreback Dec 4, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixing this
Copy link
Contributor Author

@jreback jreback Dec 4, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess its bad when I change from ValueError to TypeError and no tests break :)

It was almost all TypeError, so this is the right move.
Copy link
Contributor Author

@jreback jreback Dec 4, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually was a failed test so that is good :)
@shoyer
Copy link
Member

shoyer commented Dec 4, 2014

generally looks pretty good to me!
@shoyer shoyer mentioned this pull request Dec 4, 2014
Closed
@jreback
Copy link
Contributor Author

jreback commented Dec 4, 2014

@shoyer great!

still would love to have you refactor when you have to time to use more array broadcasting stuff.
jreback added a commit that referenced this pull request Dec 4, 2014
@jreback
Merge pull request #8981 from jreback/timestamp
d00e9c1 
BUG: Bug in Timestamp-Timestamp not returning a Timedelta type (GH8865)
@jreback jreback merged commit d00e9c1 into pandas-dev:master Dec 4, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug Datetime Datetime data dtype Dtype Conversions Unexpected or buggy dtype conversions Timedelta Timedelta data type

2 participants

@jreback @shoyer