Make to_string match pandas (drop repr-style truncation/footer) - #22929
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughIndexedFrame.to_string is removed; DataFrame.to_string and Series.to_string are added with full pandas parameter support and pandas delegation. Tests are refactored to pass truncation arguments explicitly and compute expected output from pandas; expected-failure entries are updated. ChangesDataFrame and Series to_string pandas delegation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@python/cudf/cudf/core/series.py`:
- Around line 852-871: The docstring for the Series.to_string method is
incomplete and does not document the parameters (such as buf, max_rows, etc.) or
return value semantics. Add a Parameters section listing all accepted arguments
and their descriptions, including clarification that the buf parameter controls
whether the method returns a string or None. Add a Returns section documenting
the return type and behavior, explicitly noting that when buf is provided, the
method returns None, and when buf is None (the default), it returns a formatted
string. Ensure the docstring follows pandas-style documentation conventions to
maintain consistency with the codebase.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 90d8b2e4-dc39-4d69-b948-3206e0042ccf
📒 Files selected for processing (6)
python/cudf/cudf/core/dataframe.pypython/cudf/cudf/core/indexed_frame.pypython/cudf/cudf/core/series.pypython/cudf/cudf/pandas/scripts/pandas-testing-plugin.pypython/cudf/cudf/tests/dataframe/methods/test_to_string.pypython/cudf/cudf/tests/series/methods/test_to_string.py
💤 Files with no reviewable changes (1)
- python/cudf/cudf/core/indexed_frame.py
✅ Files skipped from review due to trivial changes (1)
- python/cudf/cudf/tests/series/methods/test_to_string.py
🚧 Files skipped from review as they are similar to previous changes (2)
- python/cudf/cudf/tests/dataframe/methods/test_to_string.py
- python/cudf/cudf/pandas/scripts/pandas-testing-plugin.py
|
/merge |
…IDIA#22929) Makes `Series.to_string()` / `DataFrame.to_string()` match pandas, fixing the `cudf.pandas` failures in `tests/io/formats/test_to_string.py` (16 cases). Previously cuDF's `to_string` was just `str(self)` (i.e. `repr`), so it took no arguments, truncated based on `display.max_rows`/`display.max_columns`, rendered nulls as `<NA>`, and appended a `dtype`/`name`/`length` footer. pandas' `to_string` does none of these by default and accepts a rich set of formatting arguments. `IndexedFrame.to_string` now accepts the pandas signature and delegates to pandas' formatter (`self.to_pandas().to_string(*args, **kwargs)`), so cuDF prints identically to pandas — as the docstring already promised. ### Behavior change `to_string()` no longer truncates based on the `display.max_rows`/`display.max_columns` options (use the `max_rows`/`max_cols` arguments, matching pandas) and no longer appends the `dtype`/`name`/`length` footer or renders nulls as `<NA>` by default. `repr` / `__repr__` are unchanged, so interactive display still truncates and shows the footer as before. cuDF tests that encoded the old repr-style `to_string` output are updated to the pandas-matching behavior (passing `max_rows`/`max_cols` explicitly where truncation is intended), and two tests are added to lock in the new contract (`test_to_string_ignores_display_options`, `test_series_to_string_no_dtype_footer`). The now-passing `test_to_string.py` xfail entries are removed from the cudf.pandas plugin. ### Verification - pandas `test_to_string.py` under cudf.pandas (with plugin): 93 passed, 1 xfailed, 0 failed, 0 XPASS. - cuDF `to_string` + repr unit suites: pass (repr unchanged). - Broad regression across all dataframe/series methods + other `to_string` callers: 12496 passed, 0 failed. Authors: - GALI PREM SAGAR (https://github.com/galipremsagar) Approvers: - Matthew Roeschke (https://github.com/mroeschke) URL: NVIDIA#22929
Description
Makes
Series.to_string()/DataFrame.to_string()match pandas, fixing thecudf.pandasfailures intests/io/formats/test_to_string.py(16 cases).Previously cuDF's
to_stringwas juststr(self)(i.e.repr), so it took no arguments, truncated based ondisplay.max_rows/display.max_columns, rendered nulls as<NA>, and appended adtype/name/lengthfooter. pandas'to_stringdoes none of these by default and accepts a rich set of formatting arguments.IndexedFrame.to_stringnow accepts the pandas signature and delegates to pandas' formatter (self.to_pandas().to_string(*args, **kwargs)), so cuDF prints identically to pandas — as the docstring already promised.Behavior change
to_string()no longer truncates based on thedisplay.max_rows/display.max_columnsoptions (use themax_rows/max_colsarguments, matching pandas) and no longer appends thedtype/name/lengthfooter or renders nulls as<NA>by default.repr/__repr__are unchanged, so interactive display still truncates and shows the footer as before.cuDF tests that encoded the old repr-style
to_stringoutput are updated to the pandas-matching behavior (passingmax_rows/max_colsexplicitly where truncation is intended), and two tests are added to lock in the new contract (test_to_string_ignores_display_options,test_series_to_string_no_dtype_footer). The now-passingtest_to_string.pyxfail entries are removed from the cudf.pandas plugin.Verification
test_to_string.pyunder cudf.pandas (with plugin): 93 passed, 1 xfailed, 0 failed, 0 XPASS.to_string+ repr unit suites: pass (repr unchanged).to_stringcallers: 12496 passed, 0 failed.Checklist