[FEAT] Add str.extract_all expression #2038
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2038 +/- ##
=======================================
Coverage 84.74% 84.75%
=======================================
Files 62 62
Lines 6819 6827 +8
=======================================
+ Hits 5779 5786 +7
- Misses 1040 1041 +1
|
daft/expressions/expressions.py
Outdated
| │ --- ┆ --- │ | ||
| │ Utf8 ┆ List[Utf8] │ | ||
| ╞═════════╪════════════╡ | ||
| │ 123 456 ┆ [123 456] │ |
There was a problem hiding this comment.
Oh, does our repr not do a , between values? Interesting.
There was a problem hiding this comment.
No it does, it's just that the regex (\d+) (\d+) was matching the whole string. That's my bad, it shud just be a single (\d+) that has two matches.
daft/expressions/expressions.py
Outdated
| @@ -822,6 +822,35 @@ def extract(self, pattern: str | Expression, index: int = 0) -> Expression: | |||
| pattern_expr = Expression._to_expression(pattern) | |||
| return Expression._from_pyexpr(self._expr.utf8_extract(pattern_expr._expr, index)) | |||
|
|
|||
| def extract_all(self, pattern: str | Expression, index: int = 0) -> Expression: | |||
| r"""Extracts the specified match group from all regex matches in each string in a string column. If index is 0, the entire match is returned. | |||
There was a problem hiding this comment.
Should we include also that this function always returns a list[string]?
There was a problem hiding this comment.
Yes, will include.
| index: The index of the regex match group to extract | ||
|
|
||
| Returns: | ||
| Expression: a List[Utf8] expression with the extracted regex matches |
There was a problem hiding this comment.
Can we also try adding a See Also: section here that links to .str.extract? Could be good to start doing it.
src/daft-core/src/array/ops/utf8.rs
Outdated
| }; | ||
| let flat_child = Series::try_from(( | ||
| "matches", | ||
| Box::new(matches) as Box<dyn arrow2::array::Array>, |
There was a problem hiding this comment.
Is there no matches.into() that you can use here?
There was a problem hiding this comment.
Ah there's a to_boxed() method
src/daft-core/src/array/ops/utf8.rs
Outdated
| } | ||
| // Mismatched len case: | ||
| (self_len, pattern_len) => Err(DaftError::ComputeError(format!( | ||
| "lhs and rhs have different length arrays: {self_len} vs {pattern_len}" |
There was a problem hiding this comment.
Specify that this occurred in extract, and also for the extract_all case
There was a problem hiding this comment.
Will do this for the other str kernels as well
github-actions
bot
added
the
documentation
colin-ho
force-pushed
the
colin/extract-all
branch
from
e056f1e
to
e12cc86
Compare
extract_all(self, pattern: str | Expression, index: int = 0)Extracts the specified match group from all regex matches in each string in a string column. If index is 0, the entire match is returned. If the pattern does not match or the group does not exist, a null value is returned.
As a drive by: