Skip to content
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: Signature of UDF methods #40112

Open
rhshadrach opened this issue Feb 27, 2021 · 3 comments
Open

API: Signature of UDF methods #40112

rhshadrach opened this issue Feb 27, 2021 · 3 comments
Labels
API - Consistency Internal Consistency of API/Behavior API Design Apply Apply, Aggregate, Transform, Map Enhancement Needs Discussion Requires discussion from core team before further action

Comments

@rhshadrach
Copy link
Member

There are many signatures of pandas methods that take a UDF. If I've missed methods, please let me know and will update.

Below, other_arg is a placeholder for 1 or more keyword arguments, and value is a placeholder for the default value.

Signature: func, other_arg=value, args=value, **kwargs

Signature: func=None, other_arg=value, *args, **kwargs

Signature: func, other_arg=value, *args, **kwargs

Signature: func, *args, other_arg=Value, **kwargs

Signature: func=None, *args, other_arg=Value, **kwargs

Signature: func, *args, **kwargs

Signature: arg, *args, **kwargs

@rhshadrach rhshadrach added Needs Discussion Requires discussion from core team before further action Apply Apply, Aggregate, Transform, Map API - Consistency Internal Consistency of API/Behavior labels Feb 27, 2021
@rhshadrach
Copy link
Member Author

rhshadrach commented Feb 27, 2021

Two thoughts

  • Renaming arg to func in Resampler.transform is noncontroversial.
  • IMO, the most preferred signature is func=None, *args, other_arg=value, **kwargs (where other_arg=value may not occur). With func=None, special handling will need to happen for methods that don't take a dictionary of UDFs as **kwargs (or that feature could be implemented).

As many of these methods are very heavily used, I'm worried about the impact of having a FutureWarning for so many at once. I am wondering if a gradual transition may be more user friendly.

@attack68
Copy link
Contributor

For your list:

Styler.apply(func, axis=0, subset=None, **kwargs)
Styler.applymap(func, subset=None, **kwargs)
Styler.where(cond, value, other=None, subset=None, **kwargs)
Styler.pipe(func, *args, **kwargs)

@rhshadrach
Copy link
Member Author

rhshadrach commented Feb 26, 2023

I'm now of the opinion these signatures should be:

func, args, kwargs, other_arg

Note the args / kwargs are not *args, **kwargs. While this means users will have to pass tuples / dicts to each of these respectively, it also ensures users can reliably pass arbitrary values without interfering with other args / kwargs.

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

No branches or pull requests

3 participants