You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "renato2099 (via GitHub)" <gi...@apache.org> on 2026/04/13 18:44:58 UTC
[PR] Allow plain Python literals in regexp function wrappers [datafusion-python]
renato2099 opened a new pull request, #1493:
URL: https://github.com/apache/datafusion-python/pull/1493
# Which issue does this PR close?
Closes #1446.
# Rationale for this change
Make regex functions more pythonic
# What changes are included in this PR?
Changes to regex functions such that we can improve their usability by making them more pythonic
# Are there any user-facing changes?
Regex functions are more pythonic now
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Allow plain Python literals in regexp function wrappers [datafusion-python]
Posted by "renato2099 (via GitHub)" <gi...@apache.org>.
renato2099 commented on code in PR #1493:
URL: https://github.com/apache/datafusion-python/pull/1493#discussion_r3082066605
##########
python/datafusion/functions.py:
##########
@@ -1541,13 +1547,17 @@ def regexp_replace(
>>> result.collect_column("r")[0].as_py()
'aX bX cX'
"""
- if flags is not None:
- flags = flags.expr
- return Expr(f.regexp_replace(string.expr, pattern.expr, replacement.expr, flags))
+ flags = _to_raw_literal_expr(flags) if flags is not None else None
+ pattern = _to_raw_literal_expr(pattern)
+ replacement = _to_raw_literal_expr(replacement)
+ return Expr(f.regexp_replace(string.expr, pattern, replacement, flags))
def regexp_count(
- string: Expr, pattern: Expr, start: Expr | None = None, flags: Expr | None = None
+ string: Expr,
+ pattern: Expr | str,
+ start: Expr | str | None = None,
Review Comment:
done in the latest commit
##########
python/datafusion/functions.py:
##########
@@ -1575,19 +1585,20 @@ def regexp_count(
>>> result.collect_column("c")[0].as_py()
1
"""
- if flags is not None:
- flags = flags.expr
- start = start.expr if start is not None else start
- return Expr(f.regexp_count(string.expr, pattern.expr, start, flags))
+ flags = _to_raw_literal_expr(flags) if flags is not None else None
+ start = _to_raw_literal_expr(start) if start is not None else None
+ return Expr(
+ f.regexp_count(string.expr, _to_raw_literal_expr(pattern), start, flags)
+ )
def regexp_instr(
values: Expr,
- regex: Expr,
- start: Expr | None = None,
- n: Expr | None = None,
- flags: Expr | None = None,
- sub_expr: Expr | None = None,
+ regex: Expr | str,
+ start: Expr | str | None = None,
+ n: Expr | str | None = None,
Review Comment:
done in the latest commit
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Allow plain Python literals in regexp function wrappers [datafusion-python]
Posted by "timsaucer (via GitHub)" <gi...@apache.org>.
timsaucer commented on PR #1493:
URL: https://github.com/apache/datafusion-python/pull/1493#issuecomment-4559479755
Closing in favor of the already merged #1484
This is great work! Sorry we overlapped.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Allow plain Python literals in regexp function wrappers [datafusion-python]
Posted by "ntjohnson1 (via GitHub)" <gi...@apache.org>.
ntjohnson1 commented on code in PR #1493:
URL: https://github.com/apache/datafusion-python/pull/1493#discussion_r3075923477
##########
python/datafusion/expr.py:
##########
@@ -277,6 +277,25 @@ def _iter(
return list(_iter(exprs))
+def _to_raw_literal_expr(value: Expr | Any) -> expr_internal.Expr:
Review Comment:
I think this is fine to use internally but for the public api surface we don't want to take Any. Would be nicer to be Expr | str for example on the string functions
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Allow plain Python literals in regexp function wrappers [datafusion-python]
Posted by "timsaucer (via GitHub)" <gi...@apache.org>.
timsaucer commented on code in PR #1493:
URL: https://github.com/apache/datafusion-python/pull/1493#discussion_r3078911614
##########
python/datafusion/functions.py:
##########
@@ -1575,19 +1585,20 @@ def regexp_count(
>>> result.collect_column("c")[0].as_py()
1
"""
- if flags is not None:
- flags = flags.expr
- start = start.expr if start is not None else start
- return Expr(f.regexp_count(string.expr, pattern.expr, start, flags))
+ flags = _to_raw_literal_expr(flags) if flags is not None else None
+ start = _to_raw_literal_expr(start) if start is not None else None
+ return Expr(
+ f.regexp_count(string.expr, _to_raw_literal_expr(pattern), start, flags)
+ )
def regexp_instr(
values: Expr,
- regex: Expr,
- start: Expr | None = None,
- n: Expr | None = None,
- flags: Expr | None = None,
- sub_expr: Expr | None = None,
+ regex: Expr | str,
+ start: Expr | str | None = None,
+ n: Expr | str | None = None,
Review Comment:
I think we want these as `int` not `str`
##########
python/datafusion/functions.py:
##########
@@ -1541,13 +1547,17 @@ def regexp_replace(
>>> result.collect_column("r")[0].as_py()
'aX bX cX'
"""
- if flags is not None:
- flags = flags.expr
- return Expr(f.regexp_replace(string.expr, pattern.expr, replacement.expr, flags))
+ flags = _to_raw_literal_expr(flags) if flags is not None else None
+ pattern = _to_raw_literal_expr(pattern)
+ replacement = _to_raw_literal_expr(replacement)
+ return Expr(f.regexp_replace(string.expr, pattern, replacement, flags))
def regexp_count(
- string: Expr, pattern: Expr, start: Expr | None = None, flags: Expr | None = None
+ string: Expr,
+ pattern: Expr | str,
+ start: Expr | str | None = None,
Review Comment:
I think `start` should be `Expr | int | None` not `str`
##########
python/datafusion/functions.py:
##########
@@ -1575,19 +1585,20 @@ def regexp_count(
>>> result.collect_column("c")[0].as_py()
1
"""
- if flags is not None:
- flags = flags.expr
- start = start.expr if start is not None else start
- return Expr(f.regexp_count(string.expr, pattern.expr, start, flags))
+ flags = _to_raw_literal_expr(flags) if flags is not None else None
+ start = _to_raw_literal_expr(start) if start is not None else None
+ return Expr(
+ f.regexp_count(string.expr, _to_raw_literal_expr(pattern), start, flags)
+ )
def regexp_instr(
values: Expr,
- regex: Expr,
- start: Expr | None = None,
- n: Expr | None = None,
- flags: Expr | None = None,
- sub_expr: Expr | None = None,
+ regex: Expr | str,
+ start: Expr | str | None = None,
+ n: Expr | str | None = None,
+ flags: Expr | str | None = None,
+ sub_expr: Expr | str | None = None,
Review Comment:
I think the capture group (sub_expr) is also an `int`. We should verify
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Allow plain Python literals in regexp function wrappers [datafusion-python]
Posted by "timsaucer (via GitHub)" <gi...@apache.org>.
timsaucer closed pull request #1493: Allow plain Python literals in regexp function wrappers
URL: https://github.com/apache/datafusion-python/pull/1493
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Allow plain Python literals in regexp function wrappers [datafusion-python]
Posted by "timsaucer (via GitHub)" <gi...@apache.org>.
timsaucer commented on PR #1493:
URL: https://github.com/apache/datafusion-python/pull/1493#issuecomment-4243804098
Also worth noting is I've been working on a skill that will do these in a more automated way: https://github.com/apache/datafusion-python/pull/1484 It might be worth comparing what you came up with vs the agent.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Allow plain Python literals in regexp function wrappers [datafusion-python]
Posted by "renato2099 (via GitHub)" <gi...@apache.org>.
renato2099 commented on code in PR #1493:
URL: https://github.com/apache/datafusion-python/pull/1493#discussion_r3078779034
##########
python/datafusion/expr.py:
##########
@@ -277,6 +277,25 @@ def _iter(
return list(_iter(exprs))
+def _to_raw_literal_expr(value: Expr | Any) -> expr_internal.Expr:
Review Comment:
good point 💡
addressed in latest commit
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Allow plain Python literals in regexp function wrappers [datafusion-python]
Posted by "renato2099 (via GitHub)" <gi...@apache.org>.
renato2099 commented on PR #1493:
URL: https://github.com/apache/datafusion-python/pull/1493#issuecomment-4246956239
> Also worth noting is I've been working on a skill that will do these in a more automated way: https://github.com/apache/datafusion-python/pull/1484 It might be worth comparing what you came up with vs the agent.
oh very cool! let me give that a try as well, I might need to install claude in my personal laptop because I can't install it on my work laptop :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org