Skip to content

API: Inconsistent use of apply arguments args and **kwargs #47543

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

Open
datapythonista opened this issue Jun 29, 2022 · 2 comments
Open

API: Inconsistent use of apply arguments args and **kwargs #47543

datapythonista opened this issue Jun 29, 2022 · 2 comments
Labels
API Design Apply Apply, Aggregate, Transform, Map Needs Discussion Requires discussion from core team before further action

Comments

@datapythonista
Copy link
Member

datapythonista commented Jun 29, 2022

The signature of Series.apply (and DataFrame.apply) seems inconsistent and cumbersome (see args and **kwargs:

Series.apply(func, convert_dtype=True, args=(), **kwargs)

I'd expect to have args and kwargs, or *args and **kwargs, not a mix of both. I assume this was implemented to allow convert_dtype to be passed as a positonal argument:

my_series.apply(my_func, this_is_convert_dtype)

But this seems confusing and not very intuitive:

my_series.apply(my_func, this_arg_is_for_apply, this_must_be_a_tuple_with_all_args, this_is_a_single_kwarg=True, this_is_another_kwarg=False)

To me, the API would be much clear if it was implemented as:

Series.apply(func, *, convert_dtype=True, args=(), kwargs={})

# or

Series.apply(func, convert_dtype=True, *, args=(), kwargs={})

And I think this can be easily done in two steps, warning the user first without breaking code. If there is agreement, I'd also do the same for the rest of the methods using this same pattern.

@datapythonista datapythonista added API Design Needs Discussion Requires discussion from core team before further action Apply Apply, Aggregate, Transform, Map labels Jun 29, 2022
@attack68
Copy link
Contributor

link for relevancy #39987

@rhshadrach
Copy link
Member

rhshadrach commented Jul 4, 2022

I think this is a duplicate of #40112

Edit: on second reading, it is a bit different, but should be taken into consideration in that issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Design Apply Apply, Aggregate, Transform, Map Needs Discussion Requires discussion from core team before further action
Projects
None yet
Development

No branches or pull requests

3 participants