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

DOC: Questions on Usage of *args, **kwargs, and inplace parameters in pandas.Series.cat methods #59783

Closed
1 task done
saldanhad opened this issue Sep 12, 2024 · 2 comments · Fixed by #59878
Closed
1 task done
Assignees
Labels
Accessors accessor registration mechanism (not .str, .dt, .cat) Docs

Comments

@saldanhad
Copy link
Contributor

saldanhad commented Sep 12, 2024

Pandas version checks

  • I have checked that the issue still exists on the latest versions of the docs on main here

Location of the documentation

https://pandas.pydata.org/docs/reference/api/pandas.Series.cat.remove_categories.html#pandas.Series.cat.remove_categories

https://pandas.pydata.org/docs/reference/api/pandas.Series.cat.rename_categories.html#pandas.Series.cat.rename_categories

Documentation problem

I am currently trying to work on the issue #59592, specifically for the remove_categories and rename_categories methods. Before finalizing my changes, I need some clarification on a few points regarding the documentation:

  1. *args and **kwargs Usage:
  • For remove_categories, the method only seems to use the removals parameter.
  • Similarly, for rename_categories, *args and **kwargs are mentioned but not utilized in the method implementation. Could you clarify whether the inclusion of *args and **kwargs in the documentation for methods is by design?
  • I have noticed this pattern across many methods and wanted to confirm if this is intentional or if the documentation should be updated to reflect actual usage.
  1. inplace Parameter: The inplace parameter for methods is included in the error codes of the validation docstring. However, it is not explicitly mentioned in the current documentation, nor is it included as part of the method definition for both methods.

Suggested fix for documentation

Verify usage of **args, **kwargs, and 'inplace' if not make changes to the pandas.Series.cat methods in docs.

@saldanhad saldanhad added Docs Needs Triage Issue that has not been reviewed by a pandas team member labels Sep 12, 2024
@saldanhad saldanhad changed the title DOC: Questions on Usage of *args, **kwargs, and inplace Parameters in pandas.Series.cat methods DOC: Questions on Usage of *args, **kwargs, and inplace parameters in pandas.Series.cat methods Sep 12, 2024
@rhshadrach
Copy link
Member

rhshadrach commented Sep 14, 2024

Thanks for the report! In the pandas code, the signature of the function is

def remove_categories(self, removals) -> Self:

I believe when we wrap it for adding it to the cat accessor, the signature gets changed to *args, **kwargs as these are passed through by _create_delegator_method:

def _create_delegator_method(name: str):

I wonder if we should be using functools.wraps here. E.g.

import functools as ft
import inspect

def add_one(func):
    @ft.wraps(func)
    def wrapper(*args, **kwargs):
        args = (args[0]+1, *args[1:])
        return func(*args, **kwargs)
    return wrapper

@add_one
def foo(a, b, c=5):
    return 100 * a + 10 * b + c

print(foo(1, 2))
# 225

print(inspect.signature(foo))
# <Signature (a, b, c=5)>

Without @ft.wraps(func), you get the signature *args, **kwargs.

Further investigations and PRs to improve are welcome!

@rhshadrach rhshadrach added Accessors accessor registration mechanism (not .str, .dt, .cat) and removed Needs Triage Issue that has not been reviewed by a pandas team member labels Sep 14, 2024
@saldanhad
Copy link
Contributor Author

take

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Accessors accessor registration mechanism (not .str, .dt, .cat) Docs
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants