feat: Add support for `Expr.str.zfill` #2598

williambdean · 2025-05-23T11:16:54Z

This adds zfill for both Series and Expr

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

Related issues

Related issue #<issue number>
Closes [Enh]: str zfill #2596

Checklist

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

If you have comments or can explain your changes, please do so below

williambdean · 2025-05-23T11:17:33Z

Working through this:

check-api-reference....................................................................Failed
- hook id: check-api-reference
- exit code: 1

Series.str: not documented
{'zfill'}
Expr.str: not documented
{'zfill'}

MarcoGorelli · 2025-05-23T11:24:20Z

ah yeah you need to add it to the expr_str.md / series_str.md pages

williambdean · 2025-05-23T15:12:31Z

Thanks. On my way now.
Will add some tests when I log on later

williambdean · 2025-05-23T17:54:16Z

Is this a bug in pandas?

import pandas as pd
import pyarrow as pa

pd.Series(["A", "AB", "ABC"], dtype=pd.ArrowDtype(pa.string())).str.zfill(3)

Trackback:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/will/GitHub/various/narwhals/.venv/lib/python3.12/site-packages/pandas/core/strings/accessor.py", line 137, in wrapper
    return func(self, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/will/GitHub/various/narwhals/.venv/lib/python3.12/site-packages/pandas/core/strings/accessor.py", line 1818, in zfill
    result = self._data.array._str_map(f)
             ^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'ArrowExtensionArray' object has no attribute '_str_map'. Did you mean: '_str_pad'?

Works fine with pd.Series(["A", "AB", "ABC"], dtype="string[pyarrow]") though but that seems different dtype. Reference here

FBruzzesi

Thanks for the contribution @williambdean 🙏🏼

Is this a bug in pandas?

It looks like one 😅
We might be able to workaround by accessing the underlying pyarrow series - one example of that is the following:

narwhals/narwhals/_pandas_like/series.py

Lines 1026 to 1033 in 35ec9fe

    
           if dtype_backend == "pyarrow": 
        
               import pyarrow.compute as pc 
        
               from narwhals._arrow.utils import native_to_narwhals_dtype 
        
               ca = native.array._pa_array 
        
               result_arr = cast("ChunkedArrayAny", pc.logb(ca, base)) 
        
               nw_dtype = native_to_narwhals_dtype(result_arr.type, self._version)

it might still be worth to report it upstream

tests/expr_and_series/str/zfill_test.py

williambdean · 2025-05-23T20:20:32Z

I've created a pandas issue here pandas-dev/pandas#61485

williambdean · 2025-05-23T20:30:36Z

How do y'all do the CASE WHEN. Are there any examples? Or maybe you have a different way of implementing in mind

FBruzzesi · 2025-05-23T20:41:44Z

How do y'all do the CASE WHEN. Are there any examples?

@williambdean sure there are a lot of cases and it differs from backend to backend - #2549 has examples for lazy backends

Or maybe you have a different way of implementing in mind

Are you referring to the trailing dash comment?
If so, I guess it depends 😅 pandas already does that out of the box - I have no clue for the other backends though

williambdean · 2025-05-23T20:48:20Z

In the cases with no implementation of zfill (which seems like a few), I was using lpad. I was thinking on aiming for
CASE WHEN starts_with(col, "+") THEN ... WHEN starts_with("-") THEN ... ELSE END

what are your thoughts?

FBruzzesi · 2025-05-23T20:59:24Z

In the cases with no implementation of zfill (which seems like a few), I was using lpad. I was thinking on aiming for CASE WHEN starts_with(col, "+") THEN ... WHEN starts_with("-") THEN ... ELSE END

what are your thoughts?

Yes it seems very reasonable to do so 🙌🏼 - the log PR have a similar case of 2-3 conditions needed to be checked

williambdean · 2025-05-23T21:05:42Z

Awesome. Thanks for the lead. Will look into that example

MarcoGorelli · 2025-05-24T12:42:59Z

IMHO it's OK to xfail a pandas test here, it looks like the rest is mostly ready? we can always make a follow-up to work around the pandas bug if necessary

williambdean · 2025-05-24T12:45:16Z

Sounds good. The case that I will finish with is the behavior with leading + or - for the implementations that are using lpad

williambdean · 2025-05-24T15:57:11Z

I've added a test case for the leading +. zfill from python does "+1".zfill(3) = "+01". But it seems like the polars eager does something different

These are the current failures with the latest commits:


===================================================== short test summary info =====================================================
FAILED tests/expr_and_series/str/zfill_test.py::test_str_zfill_series[pyarrow] - AssertionError: Mismatch at index 0: 0-1 != -01
FAILED tests/expr_and_series/str/zfill_test.py::test_str_zfill[pyarrow] - AssertionError: Mismatch at index 0: 0-1 != -01
FAILED tests/expr_and_series/str/zfill_test.py::test_str_zfill[pandas[pyarrow]] - AttributeError: 'ArrowExtensionArray' object has no attribute '_str_map'. Did you mean: '_str_pad'?
FAILED tests/expr_and_series/str/zfill_test.py::test_str_zfill_series[polars[eager]] - AssertionError: Mismatch at index 1: 0+1 != +01
FAILED tests/expr_and_series/str/zfill_test.py::test_str_zfill[polars[eager]] - AssertionError: Mismatch at index 1: 0+1 != +01
FAILED tests/expr_and_series/str/zfill_test.py::test_str_zfill_series[pandas[pyarrow]] - AttributeError: 'ArrowExtensionArray' object has no attribute '_str_map'. Did you mean: '_str_pad'?
FAILED tests/expr_and_series/str/zfill_test.py::test_str_zfill[ibis] - AssertionError: Mismatch at index 0: 0-1 != -01
FAILED tests/expr_and_series/str/zfill_test.py::test_str_zfill[sqlframe] - AssertionError: Mismatch at index 0: 0-1 != -01

Which behavior is suppose to match?

Seeing some differences

[ins] In [4]: pd.Series(["+1", "++1"]).str.zfill(4)
Out[4]:
0    +001
1    +0+1
dtype: object

[ins] In [5]: import polars as pl

[ins] In [6]: pl.Series(["+1", "++1"]).str.zfill(4)
Out[6]:
shape: (2,)
Series: '' [str]
[
        "00+1"
        "0++1"
]

[ins] In [7]: "+1".zfill(4)
Out[7]: '+001'

[ins] In [8]: "++1".zfill(4)
Out[8]: '+0+1'

MarcoGorelli · 2025-05-24T16:11:40Z

Probably good to match polars

williambdean · 2025-05-24T16:29:09Z

well pandas and polars don't match 😆

MarcoGorelli · 2025-05-24T16:37:40Z

I don't really understand the pandas output, but we can standardise on what polars does and note the pandas difference in the docstring? This feels like really edge case behavior anyway

FBruzzesi

Hey @williambdean 👋🏼 thanks for your patience!

CI failures aside, I find the code in _spark_like to be the easiest to read, the most modular, variable names are self clear and the one with less repetition.

I think all other backends would benefit from a similar style of implementation 🙈

Anyway, this is definitly really close to the finish line and get into the next Monday release👌

williambdean · 2025-06-19T01:38:47Z

Thanks for that feedback. I've gone ahead and address that. Let me know your thoughts. The _arrow is a bit tough and has me running into "too many local variables"

williambdean · 2025-06-19T01:40:01Z

Need to digest the typing failure still. Any tips would be appreciated!

FBruzzesi · 2025-06-19T08:10:13Z

Hey @williambdean I did a commit (cb0c30e) to address both the following:

The _arrow is a bit tough and has me running into "too many local variables"

I restructured a bit and renamed the variables

Need to digest the typing failure still. Any tips would be appreciated!

The issue was that pl.when generates an expression rather than a series. Since the test was calling that in a select context, it was not failing. The result is of the operation is:

(Pdb) native_result
<Expr ['.when(Series[a]).then(Series[a…'] at 0x7F599DD65430>

I moved to pl.Series.zip_with to make sure a Series is maintained in the output and changed the series test slightly to check that

williambdean · 2025-06-19T11:35:35Z

Thanks for those edits @FBruzzesi

FBruzzesi · 2025-06-19T12:14:38Z

@MarcoGorelli and/or @dangotbanned would you mind giving it another pass? To me it looks ready to be merged 👌🚀

dangotbanned · 2025-06-19T12:35:35Z

@MarcoGorelli and/or @dangotbanned would you mind giving it another pass? To me it looks ready to be merged 👌🚀

Sure, I'll take a look in a bit @FBruzzesi

Would've been caught by narwhals-dev#2500

dangotbanned

Thanks all, (especially @williambdean!)

I've got a few suggestions and an idea for PolarsSeries.
I think we're almost there 🎉

narwhals/_arrow/series_str.py

narwhals/_ibis/expr_str.py

dangotbanned · 2025-06-19T13:15:11Z

narwhals/_polars/series.py

+        if self._compliant_series._backend_version <= (1, 30, 0):
+            starts_with_plus = native_series.str.starts_with("+")
+            length = native_series.str.len_chars()
+            less_than_width = length < width
+
+            other = (
+                native_series.str.slice(1, length)
+                .str.zfill(width - 1)
+                .str.pad_start(width, "+")
+            )
+            native_result = native_result.zip_with(
+                mask=~(starts_with_plus & less_than_width), other=other
+            )


Might we be able to do this a bit lazier?

There's a lot of pl.Series methods that dispatch to pl.Expr on pl.DataFrame | pl.LazyFrame under the hood.
I feel like we could do something like that for performance, but also re-use our _backend_version branching logic from PolarsExpr.

Currently we evaluate at least 6 intermediate pl.Series, which to me seems like too much for an edge case 🤔

I guess we'll see if this holds up in CI 🤞

perf: Reuse PolarsExpr impl in PolarsSeries

Co-authored-by: Dan Redding <[email protected]>

williambdean · 2025-06-19T14:18:48Z

I've merged in those suggestions. Thanks @dangotbanned

Resolves narwhals-dev#2598 (comment)

dangotbanned

Thanks for the PR @williambdean 🥳

MarcoGorelli

really solid work, thanks @williambdean for sticking to this, and others for reviewing!

just a minor comment, then we'll get this into the next release

MarcoGorelli · 2025-06-19T19:05:13Z

tests/expr_and_series/str/zfill_test.py

+def skip_pandas_pyarrow(constructor: Constructor | ConstructorEager) -> None:
+    name: str = constructor.__name__
+    if name in {"pandas_pyarrow_constructor", "modin_pyarrow_constructor"}:
+        reason = (
+            "pandas with pyarrow backend doesn't support str.zfill, see "
+            "https://github.com/pandas-dev/pandas/issues/61485"
+        )
+        raise pytest.skip(reason=reason)


we usually use skip for things which don't and will never pass

here, however, we can expect that pandas[pyarrow] will eventually support zfill, so it think we should use request.applymarker(pytest.mark.xfail)

@MarcoGorelli I keep forgetting to suggest we add more tools like this in tests.utils.py

But trimmed down to just the constructor test, like

narwhals/tests/utils.py

Lines 154 to 156 in 99222f6

def is_pyarrow_windows_no_tzdata(constructor: Constructor, /) -> bool:

"""Skip test on Windows when the tz database is not configured."""

return "pyarrow" in str(constructor) and is_windows() and not windows_has_tzdata()

I will add in a similar fashion to is_pyarrow_windows_no_tzdata

EDIT: updated

FBruzzesi · 2025-06-20T10:39:56Z

Thanks again @williambdean 🙌🏼

williambdean · 2025-06-20T11:50:45Z

Thank you all for the assistance!

get pandas zfill to work

662f258

add to the api-reference

153f0f3

williambdean added 2 commits May 23, 2025 13:50

add tests for the zfill

55d77b8

implement for other backends

7ca9fcb

williambdean and others added 2 commits May 23, 2025 13:56

Merge branch 'main' into zfill

1aa802d

add the series tests

4c4c58a

williambdean marked this pull request as ready for review May 23, 2025 18:01

FBruzzesi added the enhancement New feature or request label May 23, 2025

FBruzzesi changed the title ~~Feature: zfill~~ feat: Add support for Expr.str.zfill May 23, 2025

FBruzzesi reviewed May 23, 2025

View reviewed changes

tests/expr_and_series/str/zfill_test.py Show resolved Hide resolved

add additional test cases

dda9485

Merge branch 'main' into zfill

6cdf831

williambdean added 2 commits May 24, 2025 11:54

add additional test cases

4c41e24

implement for duckdb

2e737cc

FBruzzesi reviewed Jun 18, 2025

View reviewed changes

williambdean added 2 commits June 18, 2025 21:21

fix the doctests

0514b6a

refactor logic to look like _spark_like

d717cf3

pyarrow variable naming, typing and some misc

cb0c30e

FBruzzesi and others added 2 commits June 19, 2025 10:19

pyarrow use repeat

9c97fc3

Merge branch 'main' into zfill

c742332

remove warnings

ea48128

dangotbanned self-requested a review June 19, 2025 12:50

chore(typing): Add missing zfill

a2a5eb5

Would've been caught by narwhals-dev#2500

dangotbanned reviewed Jun 19, 2025

View reviewed changes

williambdean and others added 3 commits June 19, 2025 10:17

Update narwhals/_ibis/expr_str.py

e63ced7

Co-authored-by: Dan Redding <[email protected]>

Update narwhals/_ibis/expr_str.py

fa5c65b

Co-authored-by: Dan Redding <[email protected]>

Update narwhals/_arrow/series_str.py

9aa9eb1

Co-authored-by: Dan Redding <[email protected]>

dangotbanned added 2 commits June 19, 2025 15:44

perf: Reuse PolarsExpr impl in PolarsSeries

4842eb7

Resolves narwhals-dev#2598 (comment)

Merge branch 'main' into zfill

13f3b4b

dangotbanned approved these changes Jun 19, 2025

View reviewed changes

MarcoGorelli reviewed Jun 19, 2025

View reviewed changes

williambdean added 2 commits June 19, 2025 16:46

use xfail instead

8d9e546

rename helper function

622c768

FBruzzesi merged commit 08b2896 into narwhals-dev:main Jun 20, 2025
34 checks passed

williambdean deleted the zfill branch June 20, 2025 11:50

	if dtype_backend == "pyarrow":
	import pyarrow.compute as pc

	from narwhals._arrow.utils import native_to_narwhals_dtype

	ca = native.array._pa_array
	result_arr = cast("ChunkedArrayAny", pc.logb(ca, base))
	nw_dtype = native_to_narwhals_dtype(result_arr.type, self._version)

	def is_pyarrow_windows_no_tzdata(constructor: Constructor, /) -> bool:
	"""Skip test on Windows when the tz database is not configured."""
	return "pyarrow" in str(constructor) and is_windows() and not windows_has_tzdata()

feat: Add support for Expr.str.zfill #2598

feat: Add support for Expr.str.zfill #2598

Uh oh!

Conversation

williambdean commented May 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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

Related issues

Checklist

If you have comments or can explain your changes, please do so below

Uh oh!

williambdean commented May 23, 2025

Uh oh!

MarcoGorelli commented May 23, 2025

Uh oh!

williambdean commented May 23, 2025

Uh oh!

williambdean commented May 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

FBruzzesi left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

williambdean commented May 23, 2025

Uh oh!

williambdean commented May 23, 2025

Uh oh!

FBruzzesi commented May 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

williambdean commented May 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

FBruzzesi commented May 23, 2025

Uh oh!

williambdean commented May 23, 2025

Uh oh!

MarcoGorelli commented May 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

williambdean commented May 24, 2025

Uh oh!

williambdean commented May 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MarcoGorelli commented May 24, 2025

Uh oh!

williambdean commented May 24, 2025

Uh oh!

MarcoGorelli commented May 24, 2025

Uh oh!

FBruzzesi left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

williambdean commented Jun 19, 2025

Uh oh!

williambdean commented Jun 19, 2025

Uh oh!

FBruzzesi commented Jun 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

williambdean commented Jun 19, 2025

Uh oh!

FBruzzesi commented Jun 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dangotbanned commented Jun 19, 2025

Uh oh!

dangotbanned left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dangotbanned Jun 19, 2025

Choose a reason for hiding this comment

Uh oh!

feat: Add support for `Expr.str.zfill` #2598

feat: Add support for `Expr.str.zfill` #2598

williambdean commented May 23, 2025 •

edited

Loading

williambdean commented May 23, 2025 •

edited

Loading

FBruzzesi left a comment •

edited

Loading

FBruzzesi commented May 23, 2025 •

edited

Loading

williambdean commented May 23, 2025 •

edited

Loading

MarcoGorelli commented May 24, 2025 •

edited

Loading

williambdean commented May 24, 2025 •

edited

Loading

FBruzzesi left a comment •

edited

Loading

FBruzzesi commented Jun 19, 2025 •

edited

Loading

FBruzzesi commented Jun 19, 2025 •

edited

Loading

williambdean Jun 19, 2025 •

edited

Loading