-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
Deprecate inplace in Categorical.remove_categories #37981
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
0a45fcc
4b960f5
1439e84
d4c13f6
5ac507b
dc8a8d1
1e16941
be47fec
bed9a3a
3bcdd2b
d20ca42
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 |
---|---|---|
|
@@ -326,7 +326,9 @@ def test_validate_inplace_raises(self, value): | |
cat.add_categories(new_categories=["D", "E", "F"], inplace=value) | ||
|
||
with pytest.raises(ValueError, match=msg): | ||
cat.remove_categories(removals=["D", "E", "F"], inplace=value) | ||
with tm.assert_produces_warning(FutureWarning): | ||
# issue #37643 inplace kwarg deprecated | ||
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. nit: I'd drop this comment 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. 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. yah i find these helpful |
||
cat.remove_categories(removals=["D", "E", "F"], inplace=value) | ||
|
||
with pytest.raises(ValueError, match=msg): | ||
with tm.assert_produces_warning(FutureWarning): | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -354,7 +354,10 @@ def test_remove_categories(self): | |
tm.assert_categorical_equal(res, new) | ||
|
||
# inplace == True | ||
res = cat.remove_categories("c", inplace=True) | ||
with tm.assert_produces_warning(FutureWarning): | ||
# issue #37643 inplace kwarg deprecated | ||
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. and this one 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. same here |
||
res = cat.remove_categories("c", inplace=True) | ||
|
||
tm.assert_categorical_equal(cat, new) | ||
assert res is None | ||
|
||
|
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.
i think eventually we'll need to push this up and deprecate inplace here in Categorical.replace, but that can be done in a separate step