Skip to content

fix: drop trailing separator in concat_str(..., ignore_nulls=True) for pandas-like and Dask backends - #3967

Merged
FBruzzesi merged 11 commits into
narwhals-dev:mainfrom
00200200:fix/concat-str-trailing-null-separator
Sep 20, 2026
Merged

FBruzzesi merged 11 commits into
narwhals-dev:mainfrom
00200200:fix/concat-str-trailing-null-separator

Conversation

@00200200

Copy link
Copy Markdown
Contributor

Description

In concat_str(..., ignore_nulls=True), the separator between non-null values was previously derived by checking if the preceding column had a non-null value (~null_mask[:-1]). When the subsequent column was null but the preceding column was non-null, a dangling separator was still appended before the empty string, causing outputs like 'x-1-' instead of 'x-1'.

This updates both the pandas-like and Dask implementations so that separator (i) (placed before item (i)) is included if and only if there was at least one preceding non-null value (has_prev) AND current item (i) is non-null (~null_mask[i]).

What type of PR is this? (check all applicable)

  • 🐛 Bug Fix

Related issues

AI assistance

  • AI tools were used.

Checklist

  • Code follows style guide (ruff)
  • Tests added
  • Documented the changes

@FBruzzesi FBruzzesi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hey @00200200 thanks for you contribution. I left suggestions for both the codebase and the test suite

Comment thread src/narwhals/_dask/namespace.py Resolved
Comment thread src/narwhals/_pandas_like/namespace.py Resolved
@FBruzzesi FBruzzesi changed the title fix: drop trailing separator in pandas-like and Dask concat_str with ignore_nulls (#3962) fix: drop trailing separator in concat_str(..., ignore_nulls=True) for pandas-like and Dask backends Sep 20, 2026
@FBruzzesi FBruzzesi added fix pandas-like Issue is related to pandas-like backends dask Issue is related to dask backend labels Sep 20, 2026
@00200200

Copy link
Copy Markdown
Contributor Author

Thanks for the thorough review and clean suggestions @FBruzzesi! Applied the refactoring to both pandas-like and Dask namespaces (iterative accumulation without storing intermediate series), and updated the test suite to the parameterized regression test with xfail markers for pyarrow_table (pending #3965) and polars < 1.0 (which also exhibited the trailing separator).

@FBruzzesi FBruzzesi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Amazing! Thanks @00200200 - sorry you had to face the polars v2.0rc2 breaking changes

@FBruzzesi
FBruzzesi merged commit 385fa9e into narwhals-dev:main Sep 20, 2026
41 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dask Issue is related to dask backend fix pandas-like Issue is related to pandas-like backends

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: concat_str(ignore_nulls=True) keeps the separator before a trailing null on pandas-like and Dask

2 participants