-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
BUG: preserve name in set_categories (#17509) #17517
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 33 commits
42b1c7d
3843746
4f731cf
a4ba634
b817366
aa782dd
70089a4
4c30c96
42afe67
73a6713
8f888ef
f09c9d5
8781d64
2ed3740
8192515
9007df3
72c9ebe
90ff725
9e737c8
f44a09a
3b40433
b9ab8d3
e21fe08
9730757
1749a1c
5ae2453
9f25ad9
6b4f0f2
68a2d86
a94d52a
73f5241
f6e8aa9
d4bbe27
7bb3378
7f5ec9b
ce78e6c
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 |
---|---|---|
|
@@ -57,6 +57,24 @@ def test_getitem_listlike(self): | |
expected = c[np.array([100000]).astype(np.int64)].codes | ||
tm.assert_numpy_array_equal(result, expected) | ||
|
||
@pytest.mark.parametrize( | ||
"method", | ||
[ | ||
lambda x: x.cat.set_categories([1, 2, 3]), | ||
lambda x: x.cat.reorder_categories([2, 3, 1], ordered=True), | ||
lambda x: x.cat.rename_categories([1, 2, 3]), | ||
lambda x: x.cat.remove_unused_categories(), | ||
lambda x: x.cat.remove_categories([2]), | ||
lambda x: x.cat.add_categories([4]), | ||
lambda x: x.cat.as_ordered(), | ||
lambda x: x.cat.as_unordered(), | ||
]) | ||
def test_getname_categorical_accessor(self, method): | ||
s = pd.Series([1, 2, 3], name='A').astype('category') | ||
expected = 'A' | ||
result = method(s).name | ||
assert result == expected | ||
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. Instead of doing this, construct the expected 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 Series is not expected. Only the name is expected, which is a character 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. That's because you defined your test as such. The output of We want to make sure that nothing else changes and that 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. @gfyoung In this case asserting only the name is fine I think. Assuming the actual methods are already tested separately, this tests just asserts for all of them they preserve the name. If checking the actual resulting series, all of them would need a different result, so would just over-complicate the test. |
||
|
||
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 add additional tests for the other delegated methods that this should apply to? |
||
def test_getitem_category_type(self): | ||
# GH 14580 | ||
# test iloc() on Series with Categorical data | ||
|
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.
Ah, I misread the earlier.
self
is the instance ofCategoricalAccessor
, not theSeries
like I expected. So this approach won't work.Uh oh!
There was an error while loading. Please reload this page.
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 if you modify
pandas/pandas/core/categorical.py
Line 2078 in f11bbf2
data.name
. and then modifyCategoricalAccessor.__init__
to accept a name.data
may not have a name, so maybegetattr(data, 'name', None)
. Make sense?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.
so the signature should follow like we do in
pandas.core.indexes.accessors
, e.g.(data, index, name=None)