Support cudf-polars str.zfill

[brandon-b-miller]

Closes #19035
Closes #16480

I believe this needs pola-rs/polars#22985 to pass the one remaining failing test and the column overload described here #19035 (comment)

[vyasr]

I assume that this PR will updated to leverage #19090 once that is merged?

[brandon-b-miller]

should we do this introspection?

[brandon-b-miller]

cc @mroeschke , do we usually introspect data to error at this point? If we don't, then we risk maybe producing the wrong result in some edge cases, like a negative fill value in the zfill column overload. The scalar case is easy enough to introspect and throw, but this is a whole scan.

[mroeschke]

Is this a case where we wouldn't be able to match the result (not necessarily an error) that Polars produces?

If so, yes, I think there's some precedent on introspecting the data to raise an error. We would want to do this in _validate_input though so it can raise during translation. Plus we can do this introspection on the CPU by doing a similar option on Polars objects

[brandon-b-miller]

The case this intends to match is the one where one of the fill elements in the column overload is negative, polars throws at runtime in this case. I suppose we want to match that behavior, but it's a performance hit to every call to the column overload :(

[brandon-b-miller]

Sorry @mroeschke , just getting back to this now. I've fixed up the PR which should now be passing tests, and responded to the open question above.

[mroeschke]

Could we put this in _validate_input? Raising there will enable fallback during translation.

[mroeschke]

Looks like py-polars/tests/unit/operations/namespaces/string/test_pad.py::test_str_zfill_unicode_not_respected may need to be added to the expected failures list in cudf/python/cudf_polars/cudf_polars/testing/plugin.py. Apparently Polars doesn't add zeros for unicode characters.

[brandon-b-miller]

Polars doesn't add zeros for unicode characters.

Should this be a polars issue? Who is right?

[mroeschke]

Should this be a polars issue? Who is right?

Given that Polars has a dedicated test checking that zeros aren't padded on unicode, I suppose they are "right" and ideally libcudf wouldn't pad unicode characters

[brandon-b-miller]

Should this be a polars issue? Who is right?

Given that Polars has a dedicated test checking that zeros aren't padded on unicode, I suppose they are "right" and ideally libcudf wouldn't pad unicode characters

It may be this piece of the docs:

This method is intended for padding numeric strings. If your data contains non-ASCII characters, use [pad_start()](https://docs.pola.rs/api/python/stable/reference/series/api/polars.Series.str.pad_start.html#polars.Series.str.pad_start) instead.

Ah. I wonder if we need to do something beyond xfailing here? It seems like in the wild users would be mostly zfilling numeric data but it's still dangerous to have a behavior divergence with the GPU backend. There aren't a lot of great options though. Off the top of my head we could

• extend the libcudf zfill api to ignore unicode optionally
• scan the data unnecessarily in 99% of cases to make us safe from the one edge case
• Document the issue somewhere

None of the above seem particularly amazing. I'll think on what to do in lieu of any obvious ideas I missed.

[mroeschke]

I wonder if we need to do something beyond xfailing here?

Ah OK so I suppose the "proper" solution could use pylibcudf.strings.convert.convert_fixed_point.is_fixed_point to use with copy_if_else to used the zfilled values if the values are numeric else the old values.

[brandon-b-miller]

Ah OK so I suppose the "proper" solution could use pylibcudf.strings.convert.convert_fixed_point.is_fixed_point to use with copy_if_else to used the zfilled values if the values are numeric else the old values.

Thanks- this sounds like a winner 😄

[brandon-b-miller]

It looks like polars wants to pad strings like abc as well as numeric, so we can't use is_fixed_point. But I think I might see a way forward with all_characters_of_type, will try it out.

[brandon-b-miller]

after playing around with this a bit I can't think of a way out of this without throwing for the cases where the APIs diverge. AFAICT they'll only definitely agree for a certain subset of characters in the source data.

@davidwendt whats the fastest way of checking if a column of strings contains only alphanumeric characters, spaces, or empty strings?

[Matt711]

Can you run these tests with polars 1.29? You might need to version guard these tests ie if POLARS_VERSION_LT_130

[Matt711]

hats the fastest way of checking if a column of strings contains only alphanumeric characters, spaces, or empty strings?

Maybe cudf::strings::contains_re and then cudf::reduce

[davidwendt]

... whats the fastest way of checking if a column of strings contains only alphanumeric characters, spaces, or empty strings?

I believe this API would cover everything except empty strings
https://docs.rapids.ai/api/cudf/stable/pylibcudf/api_docs/strings/char_types/#pylibcudf.strings.char_types.all_characters_of_type

[brandon-b-miller]

Added some validation and conditional xfailing based on the polars version here. We now guard for non-ascii characters in the simplest way I can think of. It's not ideal but I think we should move forward and reassess if perf concerns ever surface.

[mroeschke]

Suggested change

	5
	if not POLARS_VERSION_LT_130
	else pytest.param(5, marks=pytest.mark.xfail(reason="fixed in Polars 1.30")),
	pytest.param(5, marks=pytest.mark.xfail(POLARS_VERSION_LT_130, reason="fixed in Polars 1.30")),

(For a follow up PR if interested)

[mroeschke]

/merge