-
-
Notifications
You must be signed in to change notification settings - Fork 18k
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
REF: Add ExtensionArray.map #51809
REF: Add ExtensionArray.map #51809
Conversation
|
pandas/core/algorithms.py
Outdated
return new_values | ||
|
||
# we must convert to python types | ||
values = arr.astype(object) |
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.
copy=False?
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.
Yeah, good point.
Might need something in the extension tests? |
I've added some simple tests in the newest version. |
I've updated. |
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.
The implementation LGTM
Gentle ping. I’m intending to follow up with fixes towards #51644, and it would be great if this could be merged. |
Merge when ready @jbrockmendel |
thanks @topper-123 |
@topper-123 does any of your recent work on this close #15706? |
I'vew answered in #15706 to keep the discussion in one place. |
Working with the
.map
method is very uncomfortable in main, because of issues withExtensionArray
:ExtensionArray
is not required to have a.map
method, but may have it.ExtensionArray
subclasses, that do have amap
method have one function parameter (mapper
), while other have two parameters (mapper
andna_action
).ExtensionArray.map
inIndexOps.Mixin._map_values
. This means we can calculate.map
forExtensionArrays
indirectly, if we wrap them in aSeries
orIndex
, but we cannot rely on the ability to do it directly on theExtensionArray
.ExtensionArray
subclasses often want create their own.map
methods.map
This PR solves this by:
IndexOpsMixin._map_values
to a functionmap_array
inalgorithms
..map
method toExtensionArray
that calls the newmap_array
function. Subclasses will override this method where needed.map_array
function insideIndexOpsMixin._map_values
..map
method forExtensionArray
, i.e. always two method parameters (mapper
andna_action
), so we can reliably call the.map
method on anyExtensionArray
subclass.Note that this PR doesn't really add new functionality, because we're mostly just moving code around, though it is now possible to call the
map
method on anyExtensionarray
directly, instead of going through e.g.Series.map
.In the main branch, there are currently several
ExtensionArray
subclasses wherena_action='ignore'
doesn't work. I'm working on making it work with all of theExtensionArray
subclasses and there is already #51645, which handlesCategorical
/CategoricalIndex
and more will follow.