ENH: Let Categorical.rename_categories take a callable by topper-123 · Pull Request #18862 · pandas-dev/pandas (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation14 Commits2 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
- tests added / passed
- passes
git diff upstream/master -u -- "*.py" | flake8 --diff - whatsnew entry
This PR allows Categorical.rename_categories to take a callable as its argument.
This is useful for quickly changing the categories the same way for all categories, e.g.
pd.Categorical(['a', 'b']).rename_categories("cat_{}".format) [cat_a, cat_b] Categories (2, object): [cat_a, cat_b]
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm. minor changes.
| @@ -139,6 +139,7 @@ Other Enhancements |
|---|
| - :func:`read_excel()` has gained the ``nrows`` parameter (:issue:`16645`) |
| - :func:``DataFrame.to_json`` and ``Series.to_json`` now accept an ``index`` argument which allows the user to exclude the index from the JSON output (:issue:`17394`) |
| - ``IntervalIndex.to_tuples()`` has gained the ``na_tuple`` parameter to control whether NA is returned as a tuple of NA, or NA itself (:issue:`18756`) |
| - ``Categorical.rename_categories`` can how take a callable as its argument (:issue:`18862`) |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use :func:`Categorical.rename_categories` here
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how -> now
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, we don't have Categorical in the API docs yet, so that won't work.
:meth:`Series.cat.rename_categories`
may work (though it applies to CategoricalIndex too).
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually let's also test this for a CI and a Series.cat.rename_categories
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure I understand this about CI. I assume that will be a seperate PR? else I'd appreciate guidance.
Do you mean test Series.cat.rename_categories? Isn't it a bit odd as that's a delegation from the Categorical namespace (not at the coding computer ATM, but that's how I understand it)?
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CI = CategoricalIndex, sure its separate but makes sense to do it here (it should just work). Series.cat....IS a delegation, but we do test things there as well (for example to make sure things are passed thru). See what we have for testing and add as appropriate (or indicate that we are not testing except for basic functionaility).
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I understood CI as Continuous Integration. Sure, I can take a look at adding a test for CategoricalIndex,
| @@ -854,6 +854,10 @@ def rename_categories(self, new_categories, inplace=False): |
|---|
| are passed through and extra categories in the mapping are |
| ignored. *New in version 0.21.0*. |
| * callable : a callable that is called on all items in the old |
| categories and whose return values comprise the new categories. |
| *New in version 0.22.0*. |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use a versionadded tag
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and if you'd fix the prior one as well
| @@ -71,9 +71,14 @@ def test_rename_categories(self): |
|---|
| exp_cat = Index(["a", "b", "c"]) |
| tm.assert_index_equal(cat.categories, exp_cat) |
| res = cat.rename_categories([1, 2, 3], inplace=True) |
| # GH18862 |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add a comment (callable)
BTW, on Github, in the timeline this is added to the 0.22 milestone, but on the right side overview, it says milestone 0.23.
Seems to be something strange going on?
corrected a minor spelling error
thanks @topper-123
we are going to release 0.22 shortly with #18876 only, everything else gets moved to 0.23.