-
-
Notifications
You must be signed in to change notification settings - Fork 18.6k
BUG: make pct_change can handle the anchored freq #28664 #28681
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
Changes from all commits
a6b1a68
5eaa983
9565b40
8295022
e7579a8
910adb3
4c6433b
1c77e59
64043b2
f61e62a
1e89f71
92301ff
d4fbf04
fadc4fe
d38fff5
0c9dd60
bdbd5fd
6ec771c
bd5673d
147ca3c
8e12959
3d70ebe
5fb77cd
ceac393
78e86fc
0b0f5c0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -370,6 +370,16 @@ def test_pct_change(self, datetime_series): | |
rs, (filled / filled.shift(freq="5D") - 1).reindex_like(filled) | ||
) | ||
|
||
def test_pct_change_with_duplicate_axis(self): | ||
# GH 28664 | ||
common_idx = date_range("2019-11-14", periods=5, freq="D") | ||
result = Series(range(5), common_idx).pct_change(freq="B") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. actually can you replicate the original issue here, I think BM was used. I don't mind having B as well. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jreback I had two options, the original issue used freq BM and periods 70. I thought you also want the explicit and explainable expected, however, if I used periods 70 then the expected would be quite long and hard to explain why it is so. so two options are as follows:
both options are not hard :D There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i c, ok, then let's leave it. |
||
|
||
# the reason that the expected should be like this is documented at PR 28681 | ||
expected = Series([np.NaN, np.inf, np.NaN, np.NaN, 3.0], common_idx) | ||
|
||
tm.assert_series_equal(result, expected) | ||
|
||
def test_pct_change_shift_over_nas(self): | ||
s = Series([1.0, 1.5, np.nan, 2.5, 3.0]) | ||
|
||
|
Uh oh!
There was an error while loading. Please reload this page.