Skip to content

ENH: add na_action to Categorical.map & CategoricalIndex.map#51645

Merged
mroeschke merged 16 commits intopandas-dev:mainfrom
topper-123:categorical_map_na_Action
Mar 30, 2023
Merged

ENH: add na_action to Categorical.map & CategoricalIndex.map#51645
mroeschke merged 16 commits intopandas-dev:mainfrom
topper-123:categorical_map_na_Action

Conversation

@topper-123
Copy link
Copy Markdown
Contributor

@topper-123 topper-123 commented Feb 26, 2023

@topper-123 topper-123 force-pushed the categorical_map_na_Action branch from 5674674 to 10f8606 Compare March 4, 2023 21:47
@topper-123 topper-123 mentioned this pull request Mar 6, 2023
Merged
1 task
@topper-123 topper-123 force-pushed the categorical_map_na_Action branch 2 times, most recently from 6b461b2 to 64b7297 Compare March 14, 2023 18:44
@mroeschke mroeschke added the Categorical Categorical Data Type label Mar 17, 2023
mroeschke
mroeschke reviewed Mar 17, 2023
View reviewed changes
Comment thread .pre-commit-config.yaml
mroeschke
mroeschke reviewed Mar 17, 2023
View reviewed changes
Comment thread pandas/core/arrays/categorical.py Outdated
mroeschke
mroeschke reviewed Mar 17, 2023
View reviewed changes
Comment thread pandas/tests/arrays/categorical/test_map.py Outdated
@topper-123
Copy link
Copy Markdown
Contributor Author

topper-123 commented Mar 20, 2023

I've updated again. I noticed that the implicit default for na_action is "ignore" for Categorical.map, while it's None generally. This is a bit of an issue because e.g. Series.map has a default of None and that can't be changed, so there's some mismatch when calling the Categorical directly compared to doing it through Series.map.

So I've set the default value of na_action="ignore" in Categorical.map as being deprecated and set to be changed in the next version.

@topper-123 topper-123 force-pushed the categorical_map_na_Action branch from 7775e2b to dc60e20 Compare March 22, 2023 05:05
@topper-123 topper-123 mentioned this pull request Mar 22, 2023
Closed
@topper-123 topper-123 force-pushed the categorical_map_na_Action branch from 1f19c3e to dc9d119 Compare March 24, 2023 16:28
@topper-123
Copy link
Copy Markdown
Contributor Author

topper-123 commented Mar 25, 2023

I’ve simplified the implementation.

@topper-123 topper-123 force-pushed the categorical_map_na_Action branch from 1fc89b7 to be2d451 Compare March 29, 2023 07:04
@topper-123
Copy link
Copy Markdown
Contributor Author

topper-123 commented Mar 29, 2023

Ping.

mroeschke
mroeschke reviewed Mar 29, 2023
View reviewed changes
Comment thread pandas/core/apply.py Outdated
mapped = obj._map_values(mapper=f, convert=self.convert_dtype)
# apply doesn't have a `na_action` keyword and for backward compat reasons
# we need to give `na_action="ignore"` for categorical data.
# TODO: remove the `na_action="ignore"` has been removed from Categorical.
Copy link
Copy Markdown
Member

@mroeschke mroeschke Mar 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this supposed to refer to once the deprecation is enforced?

mroeschke
mroeschke reviewed Mar 29, 2023
View reviewed changes
Comment thread pandas/core/arrays/categorical.py Outdated

.. deprecated:: 2.1.0

The dault value of 'ignore' has been deprecated and will be changed to
Copy link
Copy Markdown
Member

@mroeschke mroeschke Mar 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The dault value of 'ignore' has been deprecated and will be changed to
The default value of 'ignore' has been deprecated and will be changed to

mroeschke
mroeschke reviewed Mar 30, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@mroeschke mroeschke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic looks a lot simpler thanks. Kinda unfortunate that user will automatically get a deprecation warning, but i think this is okay as map is a fairly uncommon method

@mroeschke mroeschke added this to the 2.1 milestone Mar 30, 2023
@mroeschke mroeschke merged commit 72cd001 into pandas-dev:main Mar 30, 2023
@mroeschke
Copy link
Copy Markdown
Member

mroeschke commented Mar 30, 2023

Thanks @topper-123

@topper-123 topper-123 deleted the categorical_map_na_Action branch March 30, 2023 16:29
topper-123 added a commit to topper-123/pandas that referenced this pull request Apr 1, 2023
@topper-123
ENH: add na_action to Categorical.map & CategoricalIndex.map (pandas-…
8cdef8a 
…dev#51645)

* ENH: add na_action to Categorical.map

* add GH numbers

* pre-commit issues

* map Categorical with Series

* REF: simplify .map

* pass test_map

* fix whatsnew

* cleanups

* pre-commit

* deprecate Categorical.map(na_action=ignore)

* fix docstrings

* fix rebase

* simplity implementation

* fix warn

* fix comments
@mroeschke mroeschke mentioned this pull request May 7, 2024
Merged
@rhshadrach rhshadrach mentioned this pull request Oct 2, 2024
Closed
3 tasks
This was referenced Oct 2, 2024
Closed
Closed
@loicdiridollou loicdiridollou mentioned this pull request Feb 4, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mroeschke mroeschke mroeschke approved these changes

Assignees

No one assigned

Labels

Categorical Categorical Data Type

Projects

None yet

Milestone

2.1

Development

Successfully merging this pull request may close these issues.

BUG/API: Categorical.map & CategoricalIndex.map have no 'na_action' kwarg Series.map on a categorical does not process missing values

2 participants

@topper-123 @mroeschke