Skip to content

FIX-#7551: Fix name ambiguity for value_counts() on Pandas backend#7585

Merged
sfc-gh-joshi merged 12 commits intomodin-project:mainfrom
sfc-gh-vrpatel:value_counts_pandas_patch
Jun 9, 2025
Merged

FIX-#7551: Fix name ambiguity for value_counts() on Pandas backend#7585
sfc-gh-joshi merged 12 commits intomodin-project:mainfrom
sfc-gh-vrpatel:value_counts_pandas_patch

Conversation

@sfc-gh-vrpatel
Copy link
Contributor

@sfc-gh-vrpatel sfc-gh-vrpatel commented May 24, 2025

What do these changes do?

Implements index renaming for sort_rows_by_column_values() in the Pandas backend, similarly as in the Ray backend, to fix the name ambiguity error in value_counts().

@sfc-gh-vrpatel sfc-gh-vrpatel force-pushed the value_counts_pandas_patch branch from 299ef3a to 5e2fe6b Compare May 27, 2025 16:36
@sfc-gh-vrpatel sfc-gh-vrpatel marked this pull request as ready for review May 29, 2025 00:37
Copy link
Contributor

@sfc-gh-joshi sfc-gh-joshi left a comment

Choose a reason for hiding this comment

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

One minor test style thing.

Take a look at the auto-generated codecov warnings as well: it looks like there might be some parameter combinations not being covered by your tests. They might be false positives (if you set a breakpoint in one of the branches and it gets hit, then it probably is), but double check just to make sure

…ndas backend

Aims to fix a name ambiguity bug in `value_counts()` with a patch on `sort_values()` on the Pandas backend.

Signed-off-by: Vraj Patel <vraj.patel@snowflake.com>
Signed-off-by: Vraj Patel <vraj.patel@snowflake.com>
Signed-off-by: Vraj Patel <vraj.patel@snowflake.com>
Signed-off-by: Vraj Patel <vraj.patel@snowflake.com>
Signed-off-by: Vraj Patel <vraj.patel@snowflake.com>
Signed-off-by: Vraj Patel <vraj.patel@snowflake.com>
@sfc-gh-vrpatel sfc-gh-vrpatel force-pushed the value_counts_pandas_patch branch from 6992211 to a337d0e Compare June 2, 2025 22:52
Signed-off-by: Vraj Patel <vraj.patel@snowflake.com>
Signed-off-by: Vraj Patel <vraj.patel@snowflake.com>
Signed-off-by: Vraj Patel <vraj.patel@snowflake.com>
Signed-off-by: Vraj Patel <vraj.patel@snowflake.com>
Signed-off-by: Vraj Patel <vraj.patel@snowflake.com>
Copy link
Contributor

@sfc-gh-joshi sfc-gh-joshi left a comment

Choose a reason for hiding this comment

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

Changes LGTM, but it looks like CI for the Python engine is failing (https://github.com/modin-project/modin/actions/runs/15478865456/job/43580614471?pr=7585). You might need to expand the conditions for the xfail even more.

Signed-off-by: Vraj Patel <vraj.patel@snowflake.com>
@sfc-gh-joshi sfc-gh-joshi merged commit 3a24f9c into modin-project:main Jun 9, 2025
41 checks passed
sfc-gh-mvashishtha pushed a commit that referenced this pull request Jun 18, 2025
…7585)

Implements index renaming for `sort_rows_by_column_values()` in the
Pandas backend, similarly as in the Ray backend, to fix the name
ambiguity error in `value_counts()`.

Signed-off-by: Vraj Patel <vraj.patel@snowflake.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BUG: simple Series.value_counts() example raises ValueError: '0' is both an index level and a column label, which is ambiguous.

3 participants