Skip to content

Commit

Permalink
refactor(api): remove deprecated tuple syntax for order_by()
Browse files Browse the repository at this point in the history
BREAKING CHANGE: passing a tuple or a sequence of tuples to table.order_by() calls is not allowed anymore; use ibis.asc(key) or ibis.desc(key) instead
  • Loading branch information
kszucs authored and cpcloud committed Aug 7, 2023
1 parent c20ba7f commit 57733e0
Show file tree
Hide file tree
Showing 6 changed files with 3 additions and 66 deletions.
2 changes: 0 additions & 2 deletions ibis/expr/operations/tests/test_generic.py
Expand Up @@ -12,8 +12,6 @@
ValidationError,
)

# TODO(kszucs): actually we should only allow datatype classes not instances


@pytest.mark.parametrize(
("value", "dtype"),
Expand Down
21 changes: 1 addition & 20 deletions ibis/expr/types/relations.py
Expand Up @@ -1192,13 +1192,7 @@ def head(self, n: int = 5) -> Table:

def order_by(
self,
by: str
| ir.Column
| tuple[str | ir.Column, bool]
| Sequence[str]
| Sequence[ir.Column]
| Sequence[tuple[str | ir.Column, bool]]
| None,
by: str | ir.Column | Sequence[str] | Sequence[ir.Column] | None,
) -> Table:
"""Sort a table by one or more expressions.
Expand Down Expand Up @@ -1248,11 +1242,6 @@ def order_by(
│ 1 │ c │ 4 │
└───────┴────────┴───────┘
"""
used_tuple_syntax = False
if isinstance(by, tuple):
by = [by]
used_tuple_syntax = True

sort_keys = []
for item in util.promote_list(by):
if isinstance(item, tuple):
Expand All @@ -1266,14 +1255,6 @@ def order_by(
item = bind_expr(self, item)
sort_keys.append(item)

if used_tuple_syntax:
util.warn_deprecated(
"table.order_by((key, True)) and table.order_by((key, False)) syntax",
as_of="6.0",
removed_in="7.0",
instead="Use ibis.desc(key) or ibis.asc(key) instead",
)

return self.op().order_by(sort_keys).to_expr()

def union(self, table: Table, *rest: Table, distinct: bool = False) -> Table:
Expand Down
18 changes: 2 additions & 16 deletions ibis/tests/expr/test_table.py
Expand Up @@ -458,22 +458,8 @@ def test_order_by(table):
result2 = table.order_by('f').op()
assert_equal(result, result2)

with pytest.warns(FutureWarning):
result2 = table.order_by([('f', False)])
with pytest.warns(FutureWarning):
result3 = table.order_by([('f', 'descending')])
with pytest.warns(FutureWarning):
result4 = table.order_by([('f', 0)])

key2 = result2.op().sort_keys[0]
key3 = result3.op().sort_keys[0]
key4 = result4.op().sort_keys[0]

assert key2.descending is True
assert key3.descending is True
assert key4.descending is True
assert key2.expr.equals(key3.expr)
assert key2.expr.equals(key4.expr)
key2 = result2.sort_keys[0]
assert key2.descending is False


def test_order_by_desc_deferred_sort_key(table):
Expand Down

This file was deleted.

This file was deleted.

11 changes: 0 additions & 11 deletions ibis/tests/sql/test_sqlalchemy.py
Expand Up @@ -229,17 +229,6 @@ def test_order_by(star1, key, snapshot):
snapshot.assert_match(to_sql(expr), "out.sql")


@pytest.mark.parametrize(
"key",
[("f", 0), ["c", ("f", 0)]],
ids=["ascending", "mixed"],
)
def test_order_by_deprecated(star1, key, snapshot):
with pytest.warns(FutureWarning):
expr = star1.order_by(key)
snapshot.assert_match(to_sql(expr), "out.sql")


@pytest.mark.parametrize(
"expr_fn", [methodcaller("limit", 10), methodcaller("limit", 10, offset=5)]
)
Expand Down

0 comments on commit 57733e0

Please sign in to comment.