-
-
Notifications
You must be signed in to change notification settings - Fork 18.6k
BUG: Fix Series.str.zfill for ArrowDtype string arrays #61485 #61533
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
@@ -2601,6 +2601,12 @@ def _str_wrap(self, width: int, **kwargs) -> Self: | |||
result = self._apply_elementwise(predicate) | |||
return type(self)(pa.chunked_array(result)) | |||
|
|||
def _str_zfill(self, width: int) -> Self: | |||
# TODO: Replace with pc.utf8_zfill when supported by arrow |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there an actual Github issue on the arrow project about utf8_zfill
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just created one - apache/arrow#46683
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you use this solution apache/arrow#46683 (comment)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you use this solution apache/arrow#46683 (comment)?
@mroeschke That solution is not clean as it misses an important edge case ('-1', '+1'), I will be working on implementing that dedicated pc.utf8_zfill(array, width)
and it will be available to us upstream.
Meanwhile, do you suggest we keep the current implementation with a TODO
or should I change this to use pc.utf8_lpad
with some custom logic for sign handling?
bab4b9a
to
1d5c66f
Compare
82ed761
to
057db65
Compare
057db65
to
5bd175a
Compare
Implemented
_str_zfill
forArrowExtensionArray
to supportSeries.str.zfill
on Arrow-backed string arrays (ArrowDtype(pa.string())
). This fixes an AttributeError due to the method relying on_str_map
, which wasn't implemented. Used_apply_elementwise
to match the approach of other string methods. Added tests undertest_string_array.py
and confirmed they pass. Also confirmed no other relevant test files are broken and the change aligns with how other string accessors are handled.