Skip to content

Commit

Permalink
fix: Allow empty sort by columns (#18774)
Browse files Browse the repository at this point in the history
  • Loading branch information
orlp committed Sep 18, 2024
1 parent d11c5c3 commit 77b1e42
Show file tree
Hide file tree
Showing 5 changed files with 15 additions and 11 deletions.
7 changes: 6 additions & 1 deletion crates/polars-core/src/frame/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1796,7 +1796,12 @@ impl DataFrame {
slice: Option<(i64, usize)>,
) -> PolarsResult<Self> {
if by_column.is_empty() {
polars_bail!(ComputeError: "No columns selected for sorting");
// If no columns selected, any order (including original order) is correct.
return if let Some((offset, len)) = slice {
Ok(self.slice(offset, len))
} else {
Ok(self.clone())
};
}
// note that the by_column argument also contains evaluated expression from
// polars-lazy that may not even be present in this dataframe. therefore
Expand Down
4 changes: 4 additions & 0 deletions crates/polars-expr/src/expressions/sortby.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,10 @@ impl PhysicalExpr for SortByExpr {
}
fn evaluate(&self, df: &DataFrame, state: &ExecutionState) -> PolarsResult<Series> {
let series_f = || self.input.evaluate(df, state);
if self.by.is_empty() {
// Sorting by 0 columns returns input unchanged.
return series_f();
}
let (series, sorted_idx) = if self.by.len() == 1 {
let sorted_idx_f = || {
let s_sort_by = self.by[0].evaluate(df, state)?;
Expand Down
1 change: 0 additions & 1 deletion crates/polars-expr/src/planner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,6 @@ fn create_physical_expr_inner(
by,
sort_options,
} => {
polars_ensure!(!by.is_empty(), InvalidOperation: "'sort_by' got an empty set");
let phys_expr = create_physical_expr_inner(*expr, ctxt, expr_arena, schema, state)?;
let phys_by =
create_physical_expressions_from_nodes(by, ctxt, expr_arena, schema, state)?;
Expand Down
7 changes: 2 additions & 5 deletions py-polars/tests/unit/dataframe/test_df.py
Original file line number Diff line number Diff line change
Expand Up @@ -360,11 +360,8 @@ def test_sort_multi_output_exprs_01() -> None:
):
df.sort("dts", "strs", nulls_last=[True, False, True])

with pytest.raises(
ComputeError,
match="No columns selected for sorting",
):
df.sort(pl.col("^xxx$"))
# No columns selected - return original input.
assert_frame_equal(df, df.sort(pl.col("^xxx$")))


@pytest.mark.parametrize(
Expand Down
7 changes: 3 additions & 4 deletions py-polars/tests/unit/test_empty.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import pytest

import polars as pl
from polars.exceptions import InvalidOperationError
from polars.testing import assert_frame_equal, assert_series_equal


Expand Down Expand Up @@ -52,9 +51,9 @@ def test_empty_count_window() -> None:


def test_empty_sort_by_args() -> None:
df = pl.DataFrame([1, 2, 3])
with pytest.raises(InvalidOperationError):
df.select(pl.all().sort_by([]))
df = pl.DataFrame({"x": [2, 1, 3]})
assert_frame_equal(df, df.select(pl.col.x.sort_by([])))
assert_frame_equal(df, df.sort([]))


def test_empty_9137() -> None:
Expand Down

0 comments on commit 77b1e42

Please sign in to comment.