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

Fixed TYP of values argument of the pandas.DataFrame.pivot_table #893

Merged
merged 4 commits into from
Mar 21, 2024

Conversation

Aftabby
Copy link
Contributor

@Aftabby Aftabby commented Mar 20, 2024

This pull request addresses issue #885 by modifying the pivot_table stub definition in frame.pyi to allow lists of strings as the values argument. Previously, the stub definition only supported single strings or None for values.

Changes Introduced:
Updated the values argument type in the pivot_table stub definition to include _Sequence[str]. This allows lists of strings to be used for values.

Test Case:
No test case added as there was already an existing test case in the file test_frame.py on line 922 :
check(

assert_type(
df.pivot(index="col1", columns="col3", values=["col2", "col4"]),
pd.DataFrame,
),
pd.DataFrame,
)

Which already test the addressed issue and updated fix.

[ ] Closes #885

Copy link
Collaborator

@Dr-Irv Dr-Irv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to add a test for pivot_table() that resembles the code in the original issue.

@Aftabby
Copy link
Contributor Author

Aftabby commented Mar 20, 2024

Hello @Dr-Irv , I have included the test case. Can you please have a look and check if it is alright.

)
check(
assert_type(
df.pivot_table(index="col2", columns="col4", values=["col1", "col3"]), pd.DataFrame,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your test is invalid for pandas. Use this:

Suggested change
df.pivot_table(index="col2", columns="col4", values=["col1", "col3"]), pd.DataFrame,
df.pivot_table(index="col1", columns="col3", values=["col2", "col4"]), pd.DataFrame,

@Aftabby
Copy link
Contributor Author

Aftabby commented Mar 20, 2024

Changed per your suggestion. Thanks!

Copy link
Collaborator

@Dr-Irv Dr-Irv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now you are failing because you didn't install the pre-commit hooks. Please follow the instructions here: https://github.com/pandas-dev/pandas-stubs/blob/main/docs/setup.md to set up your environment.

If you then just add one space character to tests/test_frame.py, and commit, then black will reformat the code and fix the issue.

Or you can try editing by hand if you look at what black did at https://github.com/pandas-dev/pandas-stubs/actions/runs/8366596481/job/22907251773?pr=893

But that's rather risky - you're better off setting up the environment and let the tools do the work for you.

@Aftabby
Copy link
Contributor Author

Aftabby commented Mar 20, 2024

Hopefully, no more formatting issue now.

Copy link
Collaborator

@Dr-Irv Dr-Irv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @Aftabby

@Dr-Irv Dr-Irv merged commit 9b4a5a8 into pandas-dev:main Mar 21, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wrong typing for the values argument of the pandas.DataFrame.pivot_table method
2 participants