-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
API: Rename arg
to func
in Series.map
#61264
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
API: Rename arg
to func
in Series.map
#61264
Conversation
doc/source/whatsnew/v3.0.0.rst
Outdated
@@ -372,6 +372,7 @@ Other API changes | |||
- Index set operations (like union or intersection) will now ignore the dtype of | |||
an empty ``RangeIndex`` or empty ``Index`` with object dtype when determining | |||
the dtype of the resulting Index (:issue:`60797`) | |||
- Renamed the ``arg`` parameter of ``Series.map`` to ``func``. (:issue:`61260`) |
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 should be in deprecations?
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 moved it there. Since it's renaming a parameter both deprecations and API changed made sense, but I have no preference.
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.
Thanks - the way I read API changes is "things I need to change to not have my code break when upgrading". As this is a deprecation, users don't need to do that just yet.
if func is None: | ||
if "arg" in kwargs: |
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.
Noting this won't be backwards compatible with the use Series.map(arg=foo, func=myfunc)
where myfunc
is an argument of foo
. This seems unlikely enough that I am okay here.
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 are totally right. In my first implementation I was raising an exception, and then thought it would be better to allow using arg
as an argument name of the udf. If we want to be very conservative we could raise now, and stop raising when we remove the deprecation and arg
is no longer accepted as the argument name of the function.
But as you, I think is better to just let that unlikely case break compatibility.
Co-authored-by: Richard Shadrach <[email protected]>
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.
lgtm - just need to fix order of the items in the whatsnew due to my suggested change.
Thanks @datapythonista |
doc/source/whatsnew/vX.X.X.rst
file if fixing a bug or adding a new feature.CC: @rhshadrach