Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: raise if remove_colums is called with unknown column by default #852

Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions src/safeds/data/tabular/containers/_table.py
Original file line number Diff line number Diff line change
Expand Up @@ -642,15 +642,15 @@ def remove_columns(
self,
names: str | list[str],
/,
*,
ignore_unknown_names: bool = False,
) -> Table:
"""
Return a new table without the specified columns.

**Notes:**

- The original table is not modified.
- This method does not raise if a column does not exist. You can use it to ensure that the resulting table does
not contain certain columns.

Parameters
lars-reimann marked this conversation as resolved.
Show resolved Hide resolved
----------
Expand Down Expand Up @@ -691,6 +691,9 @@ def remove_columns(
if isinstance(names, str):
names = [names]

if not ignore_unknown_names:
_check_columns_exist(self, names)

return Table._from_polars_lazy_frame(
self._lazy_frame.drop(names),
)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,26 +1,67 @@
import pytest
from safeds.data.tabular.containers import Table
from safeds.exceptions import ColumnNotFoundError


# Test cases where no exception is expected
@pytest.mark.parametrize(
("table", "expected", "columns"),
("table", "expected", "columns", "ignore_unknown_names"),
[
(Table({"col1": [1, 2, 1], "col2": ["a", "b", "c"]}), Table({"col1": [1, 2, 1]}), ["col2"]),
(Table({"col1": [1, 2, 1], "col2": [1, 2, 4]}), Table(), ["col1", "col2"]),
(Table({"col1": [1, 2, 1], "col2": [1, 2, 4]}), Table({"col1": [1, 2, 1], "col2": [1, 2, 4]}), []),
(Table(), Table(), []),
(Table(), Table(), ["col1"]),
(Table({"col1": [1, 2, 1], "col2": ["a", "b", "c"]}), Table({"col1": [1, 2, 1]}), ["col2"], True),
(Table({"col1": [1, 2, 1], "col2": [1, 2, 4]}), Table(), ["col1", "col2"], True),
(Table({"col1": [1, 2, 1], "col2": [1, 2, 4]}), Table({"col1": [1, 2, 1], "col2": [1, 2, 4]}), [], True),
(Table(), Table(), [], True),
(Table(), Table(), ["col1"], True),
(Table({"col1": [1, 2, 1], "col2": ["a", "b", "c"]}), Table({"col1": [1, 2, 1]}), ["col2"], False),
(Table({"col1": [1, 2, 1], "col2": [1, 2, 4]}), Table(), ["col1", "col2"], False),
(
Table({"col1": [1, 2, 1], "col2": [1, 2, 4]}),
Table({"col1": [1, 2, 1], "col2": [1, 2, 4]}),
[],
False,
),
(Table(), Table(), [], False),
],
ids=[
"one column, ignore unknown names",
"multiple columns, ignore unknown names",
"no columns, ignore unknown names",
"empty, ignore unknown names",
"missing columns, ignore unknown names",
"one column",
"multiple columns",
"no columns",
"empty",
"missing columns",
],
)
def test_should_remove_table_columns(table: Table, expected: Table, columns: list[str]) -> None:
table = table.remove_columns(columns)
def test_should_remove_table_columns_no_exception(
table: Table,
expected: Table,
columns: list[str],
ignore_unknown_names: bool,
) -> None:
table = table.remove_columns(columns, ignore_unknown_names=ignore_unknown_names)
assert table.schema == expected.schema
assert table == expected
assert table.row_count == expected.row_count


# Test cases where an exception is expected
@pytest.mark.parametrize(
("table", "columns", "ignore_unknown_names"),
[
(Table(), ["col1"], False),
(Table(), ["col12"], False),
],
ids=[
"missing columns",
"missing columns",
],
)
def test_should_raise_error_for_unknown_columns(
table: Table,
columns: list[str],
ignore_unknown_names: bool,
) -> None:
with pytest.raises(ColumnNotFoundError):
table.remove_columns(columns, ignore_unknown_names=ignore_unknown_names)
Loading