Skip to content

BUG: .asfreq on resample on PeriodIndex/TimedeltaIndex are not #12928

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
wants to merge 1 commit into from

Conversation

jreback
Copy link
Contributor

@jreback jreback commented Apr 19, 2016

closes #12926

@jreback jreback added Bug Timedelta Timedelta data type labels Apr 19, 2016
@jreback jreback added this to the 0.18.1 milestone Apr 19, 2016
@jreback
Copy link
Contributor Author

jreback commented Apr 19, 2016

a tad hacky on this soln, as seems DatetimeIndex does lots of manipulations inside the binner to fix fencepost issues, while the other indexes (Period/Timedelta) don't seem to need this.

cc @MaximilianR created a structure to support common testing for resampling (the Base test class)

""" adjust our binner when upsampling """
ax = self.ax

if is_subperiod(ax.freq, self.freq):
Copy link
Contributor

Choose a reason for hiding this comment

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

FYI this could super up

Copy link
Contributor Author

Choose a reason for hiding this comment

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

doesn't work. I think there is something going on w.r.t. how the values come back for DTI vs. TDI, meaning TDI always adds a stamp on (then removes it here). There is lots of internal logic, e.g. _adjust_bin_edges that is doing things.

@max-sixty
Copy link
Contributor

Nice re the tests.

Tangential: I had a go deprecating the how keyword to make the Period stuff much simpler. I ran into issues with simple upsampling into super periods; I don't think that is possible without DatetimeIndex machinery atm. It's on my list though.

@jreback
Copy link
Contributor Author

jreback commented Apr 19, 2016

yep - I think we can still use much of the DatetimeIndexResampler machinery - just return the PeriodIndex at the end (and of course handle special cases)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Timedelta Timedelta data type
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: resample().asfreq() looses end period with TimedeltaIndex
2 participants