-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
API: fix str-accessor on CategoricalIndex #24005
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
Conversation
Hello @h-vetinari! Thanks for submitting the PR.
|
doc/source/whatsnew/v0.24.0.rst
Outdated
@@ -1254,6 +1254,7 @@ Categorical | |||
- Bug in :meth:`Categorical.take` with a user-provided ``fill_value`` not encoding the ``fill_value``, which could result in a ``ValueError``, incorrect results, or a segmentation fault (:issue:`23296`). | |||
- In meth:`Series.unstack`, specifying a ``fill_value`` not present in the categories now raises a ``TypeError`` rather than ignoring the ``fill_value`` (:issue:`23284`) | |||
- Bug when resampling :meth:`Dataframe.resample()` and aggregating on categorical data, the categorical dtype was getting lost. (:issue:`23227`) | |||
- Bug in the ``__name__`` attribute of several methods of :class:`Series.str`, which were set incorrectly (:issue:`23551`) |
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'm confused...how does this entry relate to your other changes?
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.
Deleted the wrong line when splitting up #23167...
pandas/tests/test_strings.py
Outdated
@@ -264,6 +261,7 @@ def test_api_per_method(self, box, dtype, | |||
+ ['mixed', 'mixed-integer'] * mixed_allowed) | |||
|
|||
if inferred_dtype in allowed_types: | |||
# i.a. GH 23555, GH 23556 |
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.a. = ?
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.
inter alia :)
Codecov Report
@@ Coverage Diff @@
## master #24005 +/- ##
=======================================
Coverage 42.45% 42.45%
=======================================
Files 161 161
Lines 51561 51561
=======================================
Hits 21888 21888
Misses 29673 29673
Continue to review full report at Codecov.
|
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.
looks good, small changes.
doc/source/whatsnew/v0.24.0.rst
Outdated
@@ -1254,6 +1254,7 @@ Categorical | |||
- Bug in :meth:`Categorical.take` with a user-provided ``fill_value`` not encoding the ``fill_value``, which could result in a ``ValueError``, incorrect results, or a segmentation fault (:issue:`23296`). | |||
- In meth:`Series.unstack`, specifying a ``fill_value`` not present in the categories now raises a ``TypeError`` rather than ignoring the ``fill_value`` (:issue:`23284`) | |||
- Bug when resampling :meth:`Dataframe.resample()` and aggregating on categorical data, the categorical dtype was getting lost. (:issue:`23227`) | |||
- Bug in many methods of the ``.str``-accessor, which always failed on `CategoricalIndex` (:issue:`23555`, :issue:`23556`) |
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.
use double backticks
pandas/core/strings.py
Outdated
|
||
# for category, we do the stuff on the categories, so blow it up | ||
# to the full series again | ||
# But for some operations, we have to do the stuff on the full values, | ||
# so make it possible to skip this step as the method already did this | ||
# before the transformation... | ||
if use_codes and self._is_categorical: | ||
result = take_1d(result, self._orig.cat.codes, | ||
# if self._orig is a CategoricalIndex, there is no .cat-accessor | ||
result = take_1d(result, Series(self._orig).cat.codes, |
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.
you can use copy=False
here in the Series construction
pandas/tests/test_strings.py
Outdated
@@ -264,6 +261,7 @@ def test_api_per_method(self, box, dtype, | |||
+ ['mixed', 'mixed-integer'] * mixed_allowed) | |||
|
|||
if inferred_dtype in allowed_types: | |||
# inter alia GH 23555, GH 23556 |
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.
just say xref
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.
this one
doc/source/whatsnew/v0.24.0.rst
Outdated
@@ -1256,6 +1256,7 @@ Categorical | |||
- Bug in :meth:`Categorical.take` with a user-provided ``fill_value`` not encoding the ``fill_value``, which could result in a ``ValueError``, incorrect results, or a segmentation fault (:issue:`23296`). | |||
- In meth:`Series.unstack`, specifying a ``fill_value`` not present in the categories now raises a ``TypeError`` rather than ignoring the ``fill_value`` (:issue:`23284`) | |||
- Bug when resampling :meth:`Dataframe.resample()` and aggregating on categorical data, the categorical dtype was getting lost. (:issue:`23227`) | |||
- Bug in many methods of the ``.str``-accessor, which always failed on ``CategoricalIndex`` (:issue:`23555`, :issue:`23556`) |
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.
re-word this to say CategoricalIndex.str
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.
@jreback
Hope this is more like what you were after.
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.
close enough
@jreback |
thanks @h-vetinari |
closes #23555
closes #23556
split off from #23167
git diff upstream/master -u -- "*.py" | flake8 --diff