Skip to content

Commit

Permalink
refactor(api): remove now unsupported max_lookback window attribute
Browse files Browse the repository at this point in the history
This feature was only supported by the pandas backend, but wasn\'t tested
properly and the support for it was entirely removed in the-epic-split refactor.

BREAKING CHANGE: `ibis.rows_with_max_lookback()` function and `ibis.window(max_lookback)` argument are removed
  • Loading branch information
kszucs authored and jcrist committed Feb 27, 2024
1 parent 92bb6fd commit 99dda5b
Show file tree
Hide file tree
Showing 9 changed files with 3 additions and 181 deletions.
9 changes: 0 additions & 9 deletions ibis/backends/impala/tests/test_window.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,15 +63,6 @@ def test_window_frame_specs(alltypes, window, snapshot):
assert_sql_equal(expr, snapshot)


def test_window_rows_with_max_lookback(alltypes):
mlb = ibis.rows_with_max_lookback(3, ibis.interval(days=3))
t = alltypes
w = ibis.trailing_window(mlb, order_by=t.i)
expr = t.a.sum().over(w)
with pytest.raises(NotImplementedError):
ibis.to_sql(expr, dialect="impala")


@pytest.mark.parametrize("name", ["sum", "min", "max", "mean"])
def test_cumulative_functions(alltypes, name, snapshot):
t = alltypes
Expand Down
44 changes: 0 additions & 44 deletions ibis/backends/pandas/tests/test_window.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,11 @@
import numpy as np
import pandas as pd
import pytest
from packaging.version import parse as vparse

import ibis
import ibis.expr.datatypes as dt
from ibis.backends.pandas import Backend
from ibis.backends.pandas.tests.conftest import TestConf as tm
from ibis.common.annotations import ValidationError
from ibis.legacy.udf.vectorized import reduction


Expand Down Expand Up @@ -469,48 +467,6 @@ def test_window_with_preceding_expr(index):
tm.assert_series_equal(result, expected.rename("mean"))


@pytest.mark.xfail(
condition=vparse("1.4") <= vparse(pd.__version__) < vparse("1.4.2"),
raises=ValueError,
reason="https://github.com/pandas-dev/pandas/pull/44068",
)
def test_window_with_mlb():
index = pd.date_range("20170501", "20170507")
data = np.random.randn(len(index), 3)
df = (
pd.DataFrame(data, columns=list("abc"), index=index)
.rename_axis("time")
.reset_index(drop=False)
)
client = ibis.pandas.connect({"df": df})
t = client.table("df")
rows_with_mlb = ibis.rows_with_max_lookback(5, ibis.interval(days=10))
expr = t.mutate(
sum=lambda df: df.a.sum().over(
ibis.trailing_window(rows_with_mlb, order_by="time", group_by="b")
)
)
result = expr.execute()
expected = df.set_index("time")
gb_df = (
expected.groupby(["b"])["a"]
.rolling("10d", closed="both")
.apply(lambda s: s.iloc[-5:].sum(), raw=False)
.sort_index(level=["time"])
.reset_index(drop=True)
)
expected = expected.reset_index(drop=False).assign(sum=gb_df)
tm.assert_frame_equal(result, expected)

rows_with_mlb = ibis.rows_with_max_lookback(5, 10)
with pytest.raises(ValidationError):
t.mutate(
sum=lambda df: df.a.sum().over(
ibis.trailing_window(rows_with_mlb, order_by="time")
)
)


def test_window_grouping_key_has_scope(t, df):
param = ibis.param(dt.string)
window = ibis.window(group_by=t.dup_strings + param)
Expand Down
11 changes: 0 additions & 11 deletions ibis/backends/postgres/tests/test_functions.py
Original file line number Diff line number Diff line change
Expand Up @@ -712,17 +712,6 @@ def test_rolling_window(alltypes, func, df):
tm.assert_series_equal(result, expected)


def test_rolling_window_with_mlb(alltypes):
t = alltypes
window = ibis.trailing_window(
preceding=ibis.rows_with_max_lookback(3, ibis.interval(days=5)),
order_by=t.timestamp_col,
)
expr = t["double_col"].sum().over(window)
with pytest.raises(NotImplementedError):
expr.execute()


@pytest.mark.parametrize("func", ["mean", "sum", "min", "max"])
def test_partitioned_window(alltypes, func, df):
t = alltypes
Expand Down
11 changes: 0 additions & 11 deletions ibis/backends/risingwave/tests/test_functions.py
Original file line number Diff line number Diff line change
Expand Up @@ -506,17 +506,6 @@ def test_rolling_window(alltypes, func, df):
tm.assert_series_equal(result, expected)


def test_rolling_window_with_mlb(alltypes):
t = alltypes
window = ibis.trailing_window(
preceding=ibis.rows_with_max_lookback(3, ibis.interval(days=5)),
order_by=t.timestamp_col,
)
expr = t["double_col"].sum().over(window)
with pytest.raises(NotImplementedError):
expr.execute()


@pytest.mark.parametrize("func", ["mean", "sum", "min", "max"])
@pytest.mark.xfail(
reason="Window function with empty PARTITION BY is not supported yet"
Expand Down
2 changes: 0 additions & 2 deletions ibis/backends/sql/rewrites.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,6 @@ def sort_to_select(_, **kwargs):
@replace(p.WindowFunction)
def window_function_to_window(_, **kwargs):
"""Convert a WindowFunction node to a Window node."""
if isinstance(_.frame, ops.RowsWindowFrame) and _.frame.max_lookback is not None:
raise NotImplementedError("max_lookback is not supported for SQL backends")
return Window(
how=_.frame.how,
func=_.func,
Expand Down
50 changes: 2 additions & 48 deletions ibis/expr/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import numbers
import operator
from collections import Counter
from typing import TYPE_CHECKING, Any, NamedTuple, overload
from typing import TYPE_CHECKING, Any, overload

import ibis.expr.builders as bl
import ibis.expr.datatypes as dt
Expand Down Expand Up @@ -44,7 +44,6 @@
from collections.abc import Iterable, Sequence
from pathlib import Path

import numpy as np
import pandas as pd
import pyarrow as pa

Expand Down Expand Up @@ -154,7 +153,6 @@
"read_parquet",
"row_number",
"rows_window",
"rows_with_max_lookback",
"schema",
"Schema",
"selectors",
Expand Down Expand Up @@ -1591,32 +1589,6 @@ def get_backend(expr: Expr | None = None) -> BaseBackend:
return expr._find_backend(use_default=True)


class RowsWithMaxLookback(NamedTuple):
rows: int
max_lookback: ir.IntervalValue


def rows_with_max_lookback(
rows: int | np.integer, max_lookback: ir.IntervalValue
) -> RowsWithMaxLookback:
"""Create a bound preceding value for use with trailing window functions.
Parameters
----------
rows
Number of rows
max_lookback
Maximum lookback in time
Returns
-------
RowsWithMaxLookback
A named tuple of rows and maximum look-back in time.
"""
return RowsWithMaxLookback(rows, max_lookback)


def window(
preceding=None,
following=None,
Expand Down Expand Up @@ -1658,12 +1630,6 @@ def window(
A window frame
"""
if isinstance(preceding, RowsWithMaxLookback):
max_lookback = preceding.max_lookback
preceding = preceding.rows
else:
max_lookback = None

has_rows = rows is not None
has_range = range is not None
has_between = between is not None
Expand All @@ -1673,12 +1639,7 @@ def window(
"Must only specify either `rows`, `range`, `between` or `preceding`/`following`"
)

builder = (
bl.LegacyWindowBuilder()
.group_by(group_by)
.order_by(order_by)
.lookback(max_lookback)
)
builder = bl.LegacyWindowBuilder().group_by(group_by).order_by(order_by)
if has_rows:
return builder.rows(*rows)
elif has_range:
Expand Down Expand Up @@ -1716,17 +1677,10 @@ def rows_window(preceding=None, following=None, group_by=None, order_by=None):
A window frame
"""
if isinstance(preceding, RowsWithMaxLookback):
max_lookback = preceding.max_lookback
preceding = preceding.rows
else:
max_lookback = None

return (
bl.LegacyWindowBuilder()
.group_by(group_by)
.order_by(order_by)
.lookback(max_lookback)
.preceding_following(preceding, following, how="rows")
)

Expand Down
6 changes: 0 additions & 6 deletions ibis/expr/builders.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,6 @@ class WindowBuilder(Builder):
end: Optional[RangeWindowBoundary] = None
groupings: VarTuple[Union[str, Resolver, ops.Value]] = ()
orderings: VarTuple[Union[str, Resolver, ops.Value]] = ()
max_lookback: Optional[ops.Value[dt.Interval]] = None

@attribute
def _table(self):
Expand All @@ -156,7 +155,6 @@ def _table(self):
self.end,
*self.groupings,
*self.orderings,
self.max_lookback,
)
valuerels = (v.relations for v in inputs if isinstance(v, ops.Value))
relations = frozenset().union(*valuerels)
Expand Down Expand Up @@ -227,9 +225,6 @@ def group_by(self, expr) -> Self:
def order_by(self, expr) -> Self:
return self.copy(orderings=self.orderings + util.promote_tuple(expr))

def lookback(self, value) -> Self:
return self.copy(max_lookback=value)

@annotated
def bind(self, table: Optional[ops.Relation]):
table = table or self._table
Expand All @@ -255,7 +250,6 @@ def bind_value(value):
end=self.end,
group_by=groupings,
order_by=orderings,
max_lookback=self.max_lookback,
)
elif self.how == "range":
return ops.RangeWindowFrame(
Expand Down
15 changes: 0 additions & 15 deletions ibis/expr/operations/window.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,21 +100,6 @@ class RowsWindowFrame(WindowFrame):
how = "rows"
start: Optional[WindowBoundary[dt.Integer]] = None
end: Optional[WindowBoundary] = None
max_lookback: Optional[Value[dt.Interval]] = None

def __init__(self, max_lookback, order_by, **kwargs):
if max_lookback:
# TODO(kszucs): this should belong to a timeseries extension rather than
# the core window operation
if len(order_by) != 1:
raise com.IbisTypeError(
"`max_lookback` window must be ordered by a single column"
)
if not order_by[0].dtype.is_timestamp():
raise com.IbisTypeError(
"`max_lookback` window must be ordered by a timestamp column"
)
super().__init__(max_lookback=max_lookback, order_by=order_by, **kwargs)


@public
Expand Down
36 changes: 1 addition & 35 deletions ibis/tests/expr/test_window_frames.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
from __future__ import annotations

import numpy as np
import pandas as pd
import pytest
from pytest import param

Expand All @@ -11,7 +10,7 @@
import ibis.expr.datatypes as dt
import ibis.expr.operations as ops
from ibis.common.annotations import ValidationError
from ibis.common.exceptions import IbisInputError, IbisTypeError
from ibis.common.exceptions import IbisInputError
from ibis.common.patterns import NoMatch, Pattern


Expand Down Expand Up @@ -260,12 +259,6 @@ def test_window_api_properly_determines_how():
assert ibis.window(following=3.14).how == "range"
assert ibis.window(following=3).how == "rows"

mlb1 = ibis.rows_with_max_lookback(3, ibis.interval(months=3))
mlb2 = ibis.rows_with_max_lookback(3, ibis.interval(pd.Timedelta(days=3)))
mlb3 = ibis.rows_with_max_lookback(np.int64(7), ibis.interval(months=3))
for mlb in [mlb1, mlb2, mlb3]:
assert ibis.window(mlb).how == "rows"


def test_window_api_mutually_exclusive_options():
with pytest.raises(IbisInputError):
Expand Down Expand Up @@ -300,13 +293,6 @@ def test_window_builder_methods(alltypes):
expected = ibis.window(preceding=5, following=1, group_by=t.a, order_by=[t.b, t.d])
assert w3 == expected

w4 = ibis.trailing_window(ibis.rows_with_max_lookback(3, ibis.interval(months=3)))
w5 = w4.group_by(t.a)
expected = ibis.trailing_window(
ibis.rows_with_max_lookback(3, ibis.interval(months=3)), group_by=t.a
)
assert w5 == expected


@pytest.mark.parametrize(
["method", "is_preceding"],
Expand Down Expand Up @@ -338,33 +324,13 @@ def test_window_api_preceding_following(method, is_preceding):
p4 = method(t.a).op()
assert p4.value == t.a.op()

# TODO(kszucs): support deferred


def test_window_api_trailing_range():
t = ibis.table([("col", "int64")], name="t")
w = ibis.trailing_range_window(ibis.interval(days=1), order_by="col")
w.bind(t)


def test_window_api_max_rows_with_lookback(alltypes):
t = alltypes
mlb = ibis.rows_with_max_lookback(3, ibis.interval(days=5))
window = ibis.trailing_window(mlb, order_by=t.i)

window = ibis.trailing_window(mlb)
with pytest.raises(IbisTypeError):
t.f.lag().over(window)

window = ibis.trailing_window(mlb, order_by=t.a)
with pytest.raises(IbisTypeError):
t.f.lag().over(window)

window = ibis.trailing_window(mlb, order_by=[t.i, t.a])
with pytest.raises(IbisTypeError):
t.f.lag().over(window)


@pytest.mark.parametrize(
["a", "b"],
[
Expand Down

0 comments on commit 99dda5b

Please sign in to comment.