BUG: pivot_table with overlapping values #61293
Merged
+95
−0
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Should I update the |
|
@it176131 - update v3.0.0. Also due to an erroneous commit with a large file, we've had to force-push to the main branch recently. You'll need to fix this up by removing the erroneous commit from your history as well. See the unexpected changes and file in the diff - those need to be reverted. @datapythonista - if we're going to force-push to a contributors branch, I think a comment detailing what's going on would be helpful. |
it176131
force-pushed
the
bug-pivot-table
branch
2 times, most recently
from
1509123 to
32c0b78
Compare
|
@rhshadrach I believe I've reverted the changes as requested. Please lmk if I did it incorrectly. |
|
Oh, so GitHub added the image we removed to the open PRs? That explains why it was in that PR... |
|
@rhshadrach this check appears to have failed due to a timeout—is there a way to restart it? |
rhshadrach
requested changes
Apr 19, 2025
- Added two tests, :func:`test_pivot_table_values_in_columns` and :func:`test_pivot_table_values_in_index`, to ensure that the `values` param is still used when the argument is shared between the `columns` and `values` params, and `index` and `values` params.
- Added condition to :func:`__internal_pivot_table` to aggregate `values` explicitly if `values` were passed, otherwise aggregate all remaining columns. This allows the tests :func:`test_pivot_table_values_in_columns` and :func:`test_pivot_table_values_in_index` in test_pivot_multilevel.py to pass.
- Added test :func:`test_pivot_table_values_as_two_params` to test that the updates in pivot.py result in expected results, satisfying GH issue pandas-dev#57876.
- Added GH issue comment to test :func:`test_pivot_table_values_as_two_params`.
- Combined tests :func:`test_pivot_table_values_in_columns` and :func:`test_pivot_table_values_in_index` into a single parametrized test, :func:`test_pivot_table_multiindex_values_as_two_params` to reduce duplicate setup code.
- Added GH issue pandas-dev#61292 as comment to test :func:`test_pivot_table_multiindex_values_as_two_params`.
- Simplified proposed logic in :func:`__internal_pivot_table`.
- Added GH issue numbers to new logic in :func:`__internal_pivot_table`.
- Added ignore-comment to silence mypy error in :func:`__internal_pivot_table`. - Added TODO-comment stating that the :meth:`DataFrameGroupBy.__getitem__` should be overloaded to match the pandas-stubs type declarations, informing mypy that the type is correct given `values` is a list.
- Added pivot_table bug to Bugs/Reshaping section referencing issues pandas-dev#57876 and pandas-dev#61292.
- Moved and simplified mypy comment per feedback.
- Removed comment about explicit aggregation per feedback.
- Removed param names and updated `argnames` arg per feedback in parametrized marker.
- Removed param names in favor of implicit args per feedback.
- Removed param names and updated arg for `argnames` in parametrized marker per feedback.
- Reduced `expected` assignments from two to one per feedback.
- Removed param names in favor of implicit args per feedback.
it176131
force-pushed
the
bug-pivot-table
branch
from
02dfef6 to
16c2051
Compare
mroeschke
requested changes
Apr 23, 2025
- Moved e_data, e_index, and e_cols to parametrized marker instead of declaring inside the test :func:`test_pivot_table_multiindex_values_as_two_params`.
- Moved `expected` setup to parametrized marker instead of in the test :meth:`TestPivotTable.test_pivot_table_values_as_two_params`.
- Removed walrus operator declarations in parametrized marker for test :meth:TestPivotTable.test_pivot_table_values_as_two_params`. Appears related to this mypy issue -> python/mypy#17377.
- Removed walrus operator declarations as I'm sure mypy would raise an issue with it given that it did in test_pivot.py (see commit 52cf560).
mroeschke
approved these changes
Apr 23, 2025
|
Thanks @it176131 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
valuesargument ignored when also supplied toindex/columnsinpivot_table#61292doc/source/whatsnew/vX.X.X.rstfile if fixing a bug or adding a new feature.