Skip to content

Commit f57a348

Browse files
authored
fix: negative start and stop parameter values in Series.str.slice() (#2104)
1 parent 6b0653f commit f57a348

File tree

3 files changed

+32
-23
lines changed

3 files changed

+32
-23
lines changed

tests/system/small/operations/test_strings.py

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,20 @@ def test_reverse(scalars_dfs):
236236

237237

238238
@pytest.mark.parametrize(
239-
["start", "stop"], [(0, 1), (3, 5), (100, 101), (None, 1), (0, 12), (0, None)]
239+
["start", "stop"],
240+
[
241+
(0, 1),
242+
(3, 5),
243+
(100, 101),
244+
(None, 1),
245+
(0, 12),
246+
(0, None),
247+
(None, -1),
248+
(-1, None),
249+
(-5, -1),
250+
(1, -1),
251+
(-10, 10),
252+
],
240253
)
241254
def test_slice(scalars_dfs, start, stop):
242255
scalars_df, scalars_pandas_df = scalars_dfs

third_party/bigframes_vendored/ibis/expr/rewrites.py

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -206,21 +206,26 @@ def replace_parameter(_, params, **kwargs):
206206
@replace(p.StringSlice)
207207
def lower_stringslice(_, **kwargs):
208208
"""Rewrite StringSlice in terms of Substring."""
209-
if _.end is None:
210-
return ops.Substring(_.arg, start=_.start)
211209
if _.start is None:
212-
return ops.Substring(_.arg, start=0, length=_.end)
213-
if (
214-
isinstance(_.start, ops.Literal)
215-
and isinstance(_.start.value, int)
216-
and isinstance(_.end, ops.Literal)
217-
and isinstance(_.end.value, int)
218-
):
219-
# optimization for constant values
220-
length = _.end.value - _.start.value
210+
real_start = 0
221211
else:
222-
length = ops.Subtract(_.end, _.start)
223-
return ops.Substring(_.arg, start=_.start, length=length)
212+
real_start = ops.IfElse(
213+
ops.GreaterEqual(_.start, 0),
214+
_.start,
215+
ops.Greatest((0, ops.Add(ops.StringLength(_.arg), _.start))),
216+
)
217+
218+
if _.end is None:
219+
real_end = ops.StringLength(_.arg)
220+
else:
221+
real_end = ops.IfElse(
222+
ops.GreaterEqual(_.end, 0),
223+
_.end,
224+
ops.Greatest((0, ops.Add(ops.StringLength(_.arg), _.end))),
225+
)
226+
227+
length = ops.Greatest((0, ops.Subtract(real_end, real_start)))
228+
return ops.Substring(_.arg, start=real_start, length=length)
224229

225230

226231
@replace(p.Analytic)

third_party/bigframes_vendored/ibis/expr/types/strings.py

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -96,15 +96,6 @@ def __getitem__(self, key: slice | int | ir.IntegerScalar) -> StringValue:
9696

9797
if isinstance(step, ir.Expr) or (step is not None and step != 1):
9898
raise ValueError("Step can only be 1")
99-
if start is not None and not isinstance(start, ir.Expr) and start < 0:
100-
raise ValueError(
101-
"Negative slicing not yet supported, got start value "
102-
f"of {start:d}"
103-
)
104-
if stop is not None and not isinstance(stop, ir.Expr) and stop < 0:
105-
raise ValueError(
106-
"Negative slicing not yet supported, got stop value " f"of {stop:d}"
107-
)
10899
if start is None and stop is None:
109100
return self
110101
return ops.StringSlice(self, start, stop).to_expr()

0 commit comments

Comments
 (0)