Skip to content

BUG/df.agg-with-df-with-missing-values-results-in-IndexError #58864

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Jun 5, 2024
Merged

BUG/df.agg-with-df-with-missing-values-results-in-IndexError #58864

merged 3 commits into from
Jun 5, 2024

Conversation

abeltavares
Copy link
Contributor

@abeltavares abeltavares commented May 29, 2024 •
edited
Loading

@abeltavares abeltavares force-pushed the BUG/df.agg-with-df-with-missing-values-results-in-IndexError branch from de69d12 to 5947e3e Compare May 29, 2024 21:45
@abeltavares abeltavares changed the title fix BUG/df.agg-with-df-with-missing-values-results-in-IndexError May 29, 2024
@abeltavares abeltavares force-pushed the BUG/df.agg-with-df-with-missing-values-results-in-IndexError branch from 58df051 to 092b60a Compare May 30, 2024 09:55
@rhshadrach rhshadrach added Bug Apply Apply, Aggregate, Transform, Map labels Jun 1, 2024
@abeltavares abeltavares force-pushed the BUG/df.agg-with-df-with-missing-values-results-in-IndexError branch 3 times, most recently from 7ad3684 to 3aec102 Compare June 2, 2024 21:52
@abeltavares abeltavares force-pushed the BUG/df.agg-with-df-with-missing-values-results-in-IndexError branch from 3aec102 to 8c6f34b Compare June 2, 2024 21:53
@abeltavares
Copy link
Contributor Author

Ready for review.

mroeschke
mroeschke reviewed Jun 3, 2024
View reviewed changes
doc/source/whatsnew/v3.0.0.rst Outdated
@@ -39,6 +39,7 @@ Other enhancements
- Users can globally disable any ``PerformanceWarning`` by setting the option ``mode.performance_warnings`` to ``False`` (:issue:`56920`)
- :meth:`Styler.format_index_names` can now be used to format the index and column names (:issue:`48936` and :issue:`47489`)
- :class:`.errors.DtypeWarning` improved to include column names when mixed data types are detected (:issue:`58174`)
- :meth:`DataFrame.agg` now correctly handles missing values without raising an IndexError (:issue:`58810`)
Copy link
Member

Choose a reason for hiding this comment

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

This should be in the bug fix section

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved.

mroeschke
mroeschke reviewed Jun 3, 2024
View reviewed changes
pandas/core/apply.py Outdated
Comment on lines 1827 to 1830
col_idx_order = list(Index(s.index).get_indexer(fun))
col_idx_order = [i for i in col_idx_order if 0 <= i < len(s)]
if col_idx_order:
s = s.iloc[col_idx_order]
Copy link
Member

Choose a reason for hiding this comment

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

Instead I think you can filter col_idx_order where it's equal to -1. See the get_indexer docstring

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, makes sense.

mroeschke
mroeschke reviewed Jun 3, 2024
View reviewed changes
pandas/core/apply.py Outdated
Comment on lines 1827 to 1830
col_idx_order = list(Index(s.index).get_indexer(fun))
col_idx_order = [i for i in col_idx_order if i != -1]
if col_idx_order:
s = s.iloc[col_idx_order]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
col_idx_order = list(Index(s.index).get_indexer(fun))
col_idx_order = [i for i in col_idx_order if i != -1]
if col_idx_order:
s = s.iloc[col_idx_order]
col_idx_order = Index(s.index).get_indexer(fun)
col_idx_order = col_idx_order[col_idx_order != -1]
s = s.iloc[col_idx_order]

Copy link
Contributor Author

@abeltavares abeltavares Jun 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

That won't produce the expected behavior.
Take the "A" example in the docstring without the condition it will be:

foo  NaN
aab  NaN
bar  NaN
dat  NaN

Which is wrong.

This happens because:

  • col_idx_order is determined by Index(s.index).get_indexer(fun).
  • Since s only has one value with index ["mean"] and fun = ["max"], there is no match, so col_idx_order = [-1].
  • The code s = s.iloc[col_idx_order] results in an empty Series because -1 indicates no match, producing thr wrong behaviour.

Is this not supposed to be?

Copy link
Member

Choose a reason for hiding this comment

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

Ah OK then you can add back the if not col_idx_order.empty: condition

Copy link
Contributor Author

@abeltavares abeltavares Jun 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

AttributeError: 'numpy.ndarray' object has no attribute 'empty'
The best way i find was to make it a list and check that way.

I guess we could use a boolean mask directly on the NumPy array returned by get_indexer applying only the valid indices.

  col_idx_order = Index(s.index).get_indexer(fun)
  valid_idx = col_idx_order != -1
  if valid_idx.any():
      s = s.iloc[col_idx_order[valid_idx]]

Let me know what you think.

Copy link
Member

Choose a reason for hiding this comment

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

Sure that solution works. Thanks.

mroeschke
mroeschke reviewed Jun 3, 2024
View reviewed changes
pandas/core/apply.py Outdated

# assign the new user-provided "named aggregation" as index names, and reindex
# it based on the whole user-provided names.
s.index = reordered_indexes[idx : idx + len(fun)]
if len(s) > 0:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if len(s) > 0:
if not s.empty:

@abeltavares abeltavares requested a review from mroeschke June 5, 2024 06:50
@mroeschke mroeschke added this to the 3.0 milestone Jun 5, 2024
@mroeschke mroeschke merged commit f7590e6 into pandas-dev:main Jun 5, 2024
47 checks passed
@mroeschke
Copy link
Member

Thanks @abeltavares

@abeltavares abeltavares deleted the BUG/df.agg-with-df-with-missing-values-results-in-IndexError branch June 5, 2024 19:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mroeschke mroeschke mroeschke approved these changes

Assignees
No one assigned
Labels
Apply Apply, Aggregate, Transform, Map Bug
Projects
None yet
Milestone
3.0
Development

Successfully merging this pull request may close these issues.

BUG: df.agg with df with missing values results in IndexError
3 participants
@abeltavares @mroeschke @rhshadrach