-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
BUG: 43909 - check monoticity of rolling groupby #44068
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 8 commits
0595567
247e8f3
9b76c6c
d6dd13e
a5467ed
e234f9a
f962e27
a743ff2
9f132a5
29de8c7
a4c22d1
195b254
ffcf1c4
f08495d
cf41892
ebae655
8163a16
08ee1ec
791c183
cb73fa2
08ec776
95975c3
52ea34b
5fba5ce
737f446
a3357a5
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 |
---|---|---|
|
@@ -648,6 +648,8 @@ def test_groupby_monotonic(self): | |
# GH 15130 | ||
# we don't need to validate monotonicity when grouping | ||
|
||
# GH 43909 we should raise an error here to match behaviour of non-groupby rolling. | ||
|
||
data = [ | ||
["David", "1/1/2015", 100], | ||
["David", "1/5/2015", 500], | ||
|
@@ -664,17 +666,14 @@ def test_groupby_monotonic(self): | |
df = DataFrame(data=data, columns=["name", "date", "amount"]) | ||
df["date"] = to_datetime(df["date"]) | ||
|
||
expected = ( | ||
df.set_index("date") | ||
.groupby("name") | ||
.apply(lambda x: x.rolling("180D")["amount"].sum()) | ||
) | ||
result = df.groupby("name").rolling("180D", on="date")["amount"].sum() | ||
tm.assert_series_equal(result, expected) | ||
with pytest.raises(ValueError, match=r".* must be monotonic"): | ||
df.groupby("name").rolling("180D", on="date") | ||
|
||
def test_non_monotonic(self): | ||
def test_non_monotonic_raises(self): | ||
# GH 13966 (similar to #15130, closed by #15175) | ||
|
||
# superceded by 43909 | ||
|
||
dates = date_range(start="2016-01-01 09:30:00", periods=20, freq="s") | ||
df = DataFrame( | ||
{ | ||
|
@@ -684,15 +683,8 @@ def test_non_monotonic(self): | |
} | ||
) | ||
|
||
result = df.groupby("A").rolling("4s", on="B").C.mean() | ||
expected = ( | ||
df.set_index("B").groupby("A").apply(lambda x: x.rolling("4s")["C"].mean()) | ||
) | ||
tm.assert_series_equal(result, expected) | ||
|
||
df2 = df.sort_values("B") | ||
result = df2.groupby("A").rolling("4s", on="B").C.mean() | ||
tm.assert_series_equal(result, expected) | ||
with pytest.raises(ValueError, match=r".* must be monotonic"): | ||
df.groupby("A").rolling("4s", on="B").C.mean() | ||
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. can you also add back the 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. can you do this. 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. pls do this 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. the successful groupby with rolling is the test above 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. but its not the same as what you are doing. it fails, you sort then it works. this needs testing. 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. agreed - amended in last push |
||
|
||
def test_rolling_cov_offset(self): | ||
# GH16058 | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you use a smaller number here