BUG: resample with TimedeltaIndex, fenceposts are off #22488
Conversation
pandas/tests/test_resample.py
Outdated
| # GH 13022 | ||
| index = pd.timedelta_range('00:00:00', '00:10:00', freq='5T') | ||
| df = DataFrame(data=[[1, 3], [-5, 10], [0, 0]], index=index) | ||
| result = df.resample('1T').asfreq() |
There was a problem hiding this comment.
Instead, could you construct the expected frame and use tm.assert_frame_equal?
|
Could you also add a downsampling test case as noted here: |
|
There is test |
Codecov Report
@@ Coverage Diff @@
## master #22488 +/- ##
=========================================
Coverage ? 92.04%
=========================================
Files ? 169
Lines ? 50782
Branches ? 0
=========================================
Hits ? 46741
Misses ? 4041
Partials ? 0
Continue to review full report at Codecov.
|
| else: | ||
| new_index = self.create_index(obj.index[0], obj.index[-1], | ||
| freq=freq) | ||
| new_index = self.create_index(obj.index[0], obj.index[-1], freq=freq) |
There was a problem hiding this comment.
The case noted here is covered by this test. However, it used to produce the incorrect index so tests were passing (which is incorrect). Here is the old expected index for TDI:
In [1]: obj.index.take(np.arange(0, len(obj.index), 2))
Out[1]: TimedeltaIndex(['1 days', '3 days', '5 days', '7 days', '9 days'], dtype='timedelta64[ns]', freq='2D')
| def test_resample_as_freq_with_subperiod(self): | ||
| # GH 13022 | ||
| index = timedelta_range('00:00:00', '00:10:00', freq='5T') | ||
| df = DataFrame(data={'value': [1, 5, 10]}, index=index) |
There was a problem hiding this comment.
can you add the other example in the OP here as well (or is it too duplicative)?
There was a problem hiding this comment.
The other example is duplicative of TestTimedeltaIndex.test_asfreq test. Should I add it?
| @@ -1156,16 +1156,6 @@ def _get_binner_for_time(self): | |||
|
|
|||
| def _adjust_binner_for_upsample(self, binner): | |||
There was a problem hiding this comment.
can you add a comment here (and maybe refresh the comment in same method for DTI resampling)
There was a problem hiding this comment.
Could you please clarify how I should refresh the comment for DTI? What should be instead of adjust our binner when upsampling?
There was a problem hiding this comment.
add a comment on why we are NOT adjusting anything here for TDI (e.g. just returning it directly). basically explain why DTI but not TDI needs adjustment
There was a problem hiding this comment.
Just a reminder: here is why we adjust a binner #10885
|
Travis flagged a linting error: |
pandas/core/resample.py
Outdated
| binner = binner[1:] | ||
| else: | ||
| binner = binner[:-1] | ||
| # Just returning binner directly, GH 13022 |
There was a problem hiding this comment.
can you add a bit more text here, e.g. the reason why this is true
|
Hello @discort! Thanks for updating the PR.
|
|
thanks @discort |
* master: DOC: Update link and description of the Spyder IDE in Ecosystem docs (pandas-dev#22136) BUG: resample with TimedeltaIndex, fenceposts are off (pandas-dev#22488) Use dispatch_to_series where possible (pandas-dev#22572) BLD: Fix openpyxl to 2.5.5 (pandas-dev#22601)
git diff upstream/master -u -- "*.py" | flake8 --diff