Skip to content

Fix str.replace(..., n=0) replacing every occurrence instead of none - #11545

Open
Kropiunig wants to merge 3 commits into
pydata:mainfrom
Kropiunig:fix-str-replace-n-zero
Open

Fix str.replace(..., n=0) replacing every occurrence instead of none#11545
Kropiunig wants to merge 3 commits into
pydata:mainfrom
Kropiunig:fix-str-replace-n-zero

Conversation

@Kropiunig

Copy link
Copy Markdown

Description

StringAccessor.replace documents n as "Number of replacements to make from start. Use -1 to replace all." The regex code path (the default, regex=True) forwards count=max(n, 0) to re.Pattern.sub, and re.sub treats count=0 as replace every occurrence. So n=0 collapses onto n=-1 and replaces everything instead of nothing — silently, with no error.

The literal path (regex=False) calls str.replace(pat, repl, n), which honours n=0 correctly. The same call therefore gives two different answers depending on regex:

>>> import xarray as xr
>>> da = xr.DataArray(["fooBAD__barBAD"], dims="x")

>>> da.str.replace("BAD", "", n=0).item()          # regex=True (default)
'foo__bar'                                          # expected 'fooBAD__barBAD'

>>> da.str.replace("BAD", "", n=0, regex=False).item()
'fooBAD__barBAD'                                    # correct

This is most damaging where n is array-like, which is a documented feature ("If pat, repl, or n is array-like, they are broadcast against the array and applied elementwise"). A single 0 in the broadcast n silently becomes "replace all" for that element:

>>> n = xr.DataArray([0, 1, -1], dims="y")
>>> da.str.replace("BAD[_]*", "", n=n).values
array([['foobar', 'foobarBAD', 'foobar']], dtype='<U9')
#        ^^^^^^ should be 'fooBAD__barBAD'

Two further references agree that n=0 means "no replacements": pandas.Series.str.replace(..., n=0) is a no-op for both regex=True and regex=False, and xarray's own str.split(..., maxsplit=0) already performs zero splits.

The fix special-cases n == 0 in the regex branch. Negative n still maps to "replace all" exactly as before, so no other behaviour changes.

Added test_replace_n_zero, which covers the regex path, the literal path and the broadcast case. It fails on main (2 failed / 12 passed for -k replace) and passes with this change (14 passed); the full xarray/tests/test_accessor_str.py module is 228 passed, 1 skipped.

Checklist

  • Tests added
  • User visible changes (including notable bug fixes) are documented in whats-new.rst

AI Disclosure

  • This PR contains AI-generated content.
    • I have tested any AI-generated content in my PR.
    • I take responsibility for any AI-generated content in my PR.
      Tools: Claude Code. Prompt: audit the xarray source for a provably-correct, unclaimed bug with a user-visible symptom on the default code path, then produce a minimal fix plus a regression test that fails before and passes after.

The regex branch of StringAccessor.replace passed count=max(n, 0) to
re.sub, where count=0 means 'replace every occurrence'. That collapsed
n=0 onto n=-1, so asking for zero replacements silently replaced all of
them, while the regex=False branch (str.replace) honoured n=0 correctly.

Special-case n=0 so both branches match the documented behaviour that n
is the number of replacements to make from the start and only -1 means
'replace all'.
@welcome

welcome Bot commented Aug 26, 2026

Copy link
Copy Markdown

Thank you for opening this pull request! It may take us a few days to respond here, so thank you for being patient.
If you have questions, some answers may be found in our contributing guidelines.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant