-
-
Notifications
You must be signed in to change notification settings - Fork 18.6k
API: Replace na_action parameter in Series/DataFrame/Index.map by the standard skipna #61530
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
base: main
Are you sure you want to change the base?
Changes from all commits
cd57c9d
77efdfd
4e089d5
d197e29
0474001
142a916
f8a069a
aaf91f7
e0b54be
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 |
---|---|---|
|
@@ -2541,17 +2541,17 @@ def __array_ufunc__(self, ufunc: np.ufunc, method: str, *inputs, **kwargs): | |
|
||
return arraylike.default_array_ufunc(self, ufunc, method, *inputs, **kwargs) | ||
|
||
def map(self, mapper, na_action: Literal["ignore"] | None = None): | ||
def map(self, mapper, skipna: bool = False): | ||
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. This is technically a public API since EA authors might have implemented 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. True, I thought about it, but I couldn't find any extension array that implements map, at least in the ones in our ecosystem. And if there is one I'm not aware of, it should call I'm ok to add a I addressed the other comments, thanks for the feedback. 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. OK if you surveyed other EAs in the ecosystem, I feel more comfortable making this a breaking change. Could you note it in the breaking changes section at least in 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. I was double checking. pandas-pint does implement map, but it doesn't call super, so this is not used. What I didn't see before is that it does call I'm still not sure if the warning is the best strategy, maybe I'm just overthinking. @MichaelTiemannOSC, it would be good to have your feedback about this. |
||
""" | ||
Map values using an input mapping or function. | ||
|
||
Parameters | ||
---------- | ||
mapper : function, dict, or Series | ||
Mapping correspondence. | ||
na_action : {None, 'ignore'}, default None | ||
If 'ignore', propagate NA values, without passing them to the | ||
mapping correspondence. If 'ignore' is not supported, a | ||
skipna : bool, default False | ||
If ``True``, propagate NA values, without passing them to the | ||
mapping correspondence. If ``True`` is not supported, a | ||
``NotImplementedError`` should be raised. | ||
|
||
Returns | ||
|
@@ -2561,7 +2561,7 @@ def map(self, mapper, na_action: Literal["ignore"] | None = None): | |
If the function returns a tuple with more than one element | ||
a MultiIndex will be returned. | ||
""" | ||
return map_array(self, mapper, na_action=na_action) | ||
return map_array(self, mapper, skipna=skipna) | ||
|
||
# ------------------------------------------------------------------------ | ||
# GroupBy Methods | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10641,7 +10641,11 @@ def apply( | |
raise ValueError(f"Unknown engine {engine}") | ||
|
||
def map( | ||
self, func: PythonFuncType, na_action: Literal["ignore"] | None = None, **kwargs | ||
self, | ||
func: PythonFuncType, | ||
na_action: Literal["ignore"] | None | lib.NoDefault = lib.no_default, | ||
skipna: bool = False, | ||
**kwargs, | ||
) -> DataFrame: | ||
""" | ||
Apply a function to a Dataframe elementwise. | ||
|
@@ -10659,6 +10663,11 @@ def map( | |
Python function, returns a single value from a single value. | ||
na_action : {None, 'ignore'}, default None | ||
If 'ignore', propagate NaN values, without passing them to func. | ||
|
||
.. deprecated:: 3.0.0 | ||
Use ``skipna`` instead. | ||
skipna : bool = False | ||
If ``True``, propagate missing values without passing them to ``func``. | ||
**kwargs | ||
Additional keyword arguments to pass as keywords arguments to | ||
`func`. | ||
|
@@ -10691,7 +10700,7 @@ def map( | |
|
||
>>> df_copy = df.copy() | ||
>>> df_copy.iloc[0, 0] = pd.NA | ||
>>> df_copy.map(lambda x: len(str(x)), na_action="ignore") | ||
>>> df_copy.map(lambda x: len(str(x)), skipna=True) | ||
0 1 | ||
0 NaN 4 | ||
1 5.0 5 | ||
|
@@ -10719,16 +10728,28 @@ def map( | |
0 1.000000 4.494400 | ||
1 11.262736 20.857489 | ||
""" | ||
if na_action not in {"ignore", None}: | ||
raise ValueError(f"na_action must be 'ignore' or None. Got {na_action!r}") | ||
if na_action != lib.no_default: | ||
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. In case it's not obvious, I'm using |
||
warnings.warn( | ||
"The ``na_action`` parameter has been deprecated and it will be " | ||
"removed in a future version of pandas. Use ``skipna`` instead.", | ||
FutureWarning, | ||
stacklevel=find_stack_level(), | ||
) | ||
if na_action == "ignore": | ||
skipna = True | ||
elif na_action not in (None, "ignore"): | ||
raise ValueError( | ||
"na_action must either be 'ignore' or None, " | ||
f"{na_action!r} was passed" | ||
) | ||
|
||
if self.empty: | ||
return self.copy() | ||
|
||
func = functools.partial(func, **kwargs) | ||
|
||
def infer(x): | ||
return x._map_values(func, na_action=na_action) | ||
return x._map_values(func, skipna=skipna) | ||
|
||
return self.apply(infer).__finalize__(self, "map") | ||
|
||
|
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
skipna
fixture already exists.