feat(bigframes): Transpiler supports more string ops#17693
feat(bigframes): Transpiler supports more string ops#17693TrevorBergeron wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request adds support for compiling f-strings, nullity checks (such as 'is None' and 'is not None'), and various string operations (e.g., capitalize, islower, isupper) in Python UDFs. Feedback is provided regarding the Polars compilation of 'IsLowerOp' and 'IsUpperOp', where the current regex patterns incorrectly require the entire string to consist only of letters, failing to match Python's native behavior which allows non-cased characters.
| @compile_op.register(string_ops.IsLowerOp) | ||
| def _(self, op: ops.ScalarOp, input: pl.Expr) -> pl.Expr: | ||
| assert isinstance(op, string_ops.IsLowerOp) | ||
| return input.str.contains(r"^[a-z]+$") | ||
|
|
||
| @compile_op.register(string_ops.IsUpperOp) | ||
| def _(self, op: ops.ScalarOp, input: pl.Expr) -> pl.Expr: | ||
| assert isinstance(op, string_ops.IsUpperOp) | ||
| return input.str.contains(r"^[A-Z]+$") |
There was a problem hiding this comment.
In Python, str.islower() and str.isupper() return True if there is at least one cased character and all cased characters are lowercase/uppercase respectively. Non-cased characters (such as spaces, punctuation, and digits) are allowed.
The current regex patterns ^[a-z]+$ and ^[A-Z]+$ require the entire string to consist only of letters of that case, which incorrectly returns False for valid Python strings like "hello world", "hello!", or "abc123".
To match Python's behavior, use regex lookaheads to ensure there is at least one cased character of the correct case, and no cased characters of the opposite case.
| @compile_op.register(string_ops.IsLowerOp) | |
| def _(self, op: ops.ScalarOp, input: pl.Expr) -> pl.Expr: | |
| assert isinstance(op, string_ops.IsLowerOp) | |
| return input.str.contains(r"^[a-z]+$") | |
| @compile_op.register(string_ops.IsUpperOp) | |
| def _(self, op: ops.ScalarOp, input: pl.Expr) -> pl.Expr: | |
| assert isinstance(op, string_ops.IsUpperOp) | |
| return input.str.contains(r"^[A-Z]+$") | |
| @compile_op.register(string_ops.IsLowerOp) | |
| def _(self, op: ops.ScalarOp, input: pl.Expr) -> pl.Expr: | |
| assert isinstance(op, string_ops.IsLowerOp) | |
| return input.str.contains(r"^(?=[^a-z]*[a-z])[^A-Z]+$") | |
| @compile_op.register(string_ops.IsUpperOp) | |
| def _(self, op: ops.ScalarOp, input: pl.Expr) -> pl.Expr: | |
| assert isinstance(op, string_ops.IsUpperOp) | |
| return input.str.contains(r"^(?=[^A-Z]*[A-Z])[^a-z]+$") |
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #<issue_number_goes_here> 🦕