From fb4822f9daf2d882c38340cfcf64bb604358740f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Gr=C3=A9us?= Date: Sun, 16 Apr 2023 20:03:15 +0200 Subject: [PATCH 1/7] Resolved bug #115 and added test for this explicit behaviour Fixed grammar --- src/safeds/data/tabular/containers/_table.py | 6 +++--- .../tabular/containers/_table/test_keep_only_columns.py | 9 +++++++++ 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/safeds/data/tabular/containers/_table.py b/src/safeds/data/tabular/containers/_table.py index 782ddb814..911a9c8e0 100644 --- a/src/safeds/data/tabular/containers/_table.py +++ b/src/safeds/data/tabular/containers/_table.py @@ -566,7 +566,7 @@ def keep_only_columns(self, column_names: list[str]) -> Table: Raises ------ ColumnNameError - If any of the given columns do not exist. + If any of the given columns does not exist. """ invalid_columns = [] column_indices = [] @@ -578,7 +578,7 @@ def keep_only_columns(self, column_names: list[str]) -> Table: if len(invalid_columns) != 0: raise UnknownColumnNameError(invalid_columns) transformed_data = self._data[column_indices] - transformed_data.columns = [name for name in self._schema.get_column_names() if name in column_names] + transformed_data.columns = [self._schema.get_column_names()[i] for i in column_indices] return Table(transformed_data) def remove_columns(self, column_names: list[str]) -> Table: @@ -598,7 +598,7 @@ def remove_columns(self, column_names: list[str]) -> Table: Raises ------ ColumnNameError - If any of the given columns do not exist. + If any of the given columns does not exist. """ invalid_columns = [] column_indices = [] diff --git a/tests/safeds/data/tabular/containers/_table/test_keep_only_columns.py b/tests/safeds/data/tabular/containers/_table/test_keep_only_columns.py index 35688d1b6..2c746b40d 100644 --- a/tests/safeds/data/tabular/containers/_table/test_keep_only_columns.py +++ b/tests/safeds/data/tabular/containers/_table/test_keep_only_columns.py @@ -12,6 +12,15 @@ def test_keep_columns() -> None: assert not transformed_table.schema.has_column("B") +def test_keep_columns_order() -> None: + table = Table.from_csv_file(resolve_resource_path("test_table_from_csv_file.csv")) + transformed_table = table.keep_only_columns(["B", "A"]) + assert table.to_columns()[0] == transformed_table.to_columns()[1] + assert table.to_columns()[1] == transformed_table.to_columns()[0] + assert table.get_column("A") == transformed_table.get_column("A") + assert table.get_column("B") == transformed_table.get_column("B") + + def test_keep_columns_warning() -> None: table = Table.from_csv_file(resolve_resource_path("test_table_from_csv_file.csv")) with pytest.raises(UnknownColumnNameError): From 9fdd5b383a9612e0b784314200a257f22dda869e Mon Sep 17 00:00:00 2001 From: Alexander <47296670+Marsmaennchen221@users.noreply.github.com> Date: Sun, 16 Apr 2023 21:48:33 +0200 Subject: [PATCH 2/7] Apply suggestions from code review Co-authored-by: Lars Reimann --- src/safeds/data/tabular/containers/_table.py | 2 +- .../data/tabular/containers/_table/test_keep_only_columns.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/safeds/data/tabular/containers/_table.py b/src/safeds/data/tabular/containers/_table.py index 911a9c8e0..9747d5813 100644 --- a/src/safeds/data/tabular/containers/_table.py +++ b/src/safeds/data/tabular/containers/_table.py @@ -578,7 +578,7 @@ def keep_only_columns(self, column_names: list[str]) -> Table: if len(invalid_columns) != 0: raise UnknownColumnNameError(invalid_columns) transformed_data = self._data[column_indices] - transformed_data.columns = [self._schema.get_column_names()[i] for i in column_indices] + transformed_data.columns = column_names return Table(transformed_data) def remove_columns(self, column_names: list[str]) -> Table: diff --git a/tests/safeds/data/tabular/containers/_table/test_keep_only_columns.py b/tests/safeds/data/tabular/containers/_table/test_keep_only_columns.py index 2c746b40d..67be1a99f 100644 --- a/tests/safeds/data/tabular/containers/_table/test_keep_only_columns.py +++ b/tests/safeds/data/tabular/containers/_table/test_keep_only_columns.py @@ -12,7 +12,7 @@ def test_keep_columns() -> None: assert not transformed_table.schema.has_column("B") -def test_keep_columns_order() -> None: +def test_keep_only_columns_order() -> None: table = Table.from_csv_file(resolve_resource_path("test_table_from_csv_file.csv")) transformed_table = table.keep_only_columns(["B", "A"]) assert table.to_columns()[0] == transformed_table.to_columns()[1] @@ -21,7 +21,7 @@ def test_keep_columns_order() -> None: assert table.get_column("B") == transformed_table.get_column("B") -def test_keep_columns_warning() -> None: +def test_keep_only_columns_warning() -> None: table = Table.from_csv_file(resolve_resource_path("test_table_from_csv_file.csv")) with pytest.raises(UnknownColumnNameError): table.keep_only_columns(["C"]) From fc71c37166418979f9666192819cf63c9d5660d2 Mon Sep 17 00:00:00 2001 From: Lars Reimann Date: Sun, 16 Apr 2023 22:03:49 +0200 Subject: [PATCH 3/7] test: rename test function to match name of tested function --- .../data/tabular/containers/_table/test_keep_only_columns.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/safeds/data/tabular/containers/_table/test_keep_only_columns.py b/tests/safeds/data/tabular/containers/_table/test_keep_only_columns.py index 67be1a99f..f6deb9f85 100644 --- a/tests/safeds/data/tabular/containers/_table/test_keep_only_columns.py +++ b/tests/safeds/data/tabular/containers/_table/test_keep_only_columns.py @@ -5,7 +5,7 @@ from tests.helpers import resolve_resource_path -def test_keep_columns() -> None: +def test_keep_only_columns() -> None: table = Table.from_csv_file(resolve_resource_path("test_table_from_csv_file.csv")) transformed_table = table.keep_only_columns(["A"]) assert transformed_table.schema.has_column("A") From 2bf303adc75053d98d0abdcdb7dd8e05f5d2b562 Mon Sep 17 00:00:00 2001 From: Lars Reimann Date: Sun, 16 Apr 2023 22:07:44 +0200 Subject: [PATCH 4/7] test: specify expected result explicitly --- .../containers/_table/test_keep_only_columns.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/tests/safeds/data/tabular/containers/_table/test_keep_only_columns.py b/tests/safeds/data/tabular/containers/_table/test_keep_only_columns.py index f6deb9f85..0d4b7f5ac 100644 --- a/tests/safeds/data/tabular/containers/_table/test_keep_only_columns.py +++ b/tests/safeds/data/tabular/containers/_table/test_keep_only_columns.py @@ -1,5 +1,5 @@ import pytest -from safeds.data.tabular.containers import Table +from safeds.data.tabular.containers import Table, Column from safeds.data.tabular.exceptions import UnknownColumnNameError from tests.helpers import resolve_resource_path @@ -15,10 +15,12 @@ def test_keep_only_columns() -> None: def test_keep_only_columns_order() -> None: table = Table.from_csv_file(resolve_resource_path("test_table_from_csv_file.csv")) transformed_table = table.keep_only_columns(["B", "A"]) - assert table.to_columns()[0] == transformed_table.to_columns()[1] - assert table.to_columns()[1] == transformed_table.to_columns()[0] - assert table.get_column("A") == transformed_table.get_column("A") - assert table.get_column("B") == transformed_table.get_column("B") + assert transformed_table == Table.from_columns( + [ + Column("B", [2]), + Column("A", [1]) + ] + ) def test_keep_only_columns_warning() -> None: From e23df8b3ebbeeccac761e8fe8a928625e26eb328 Mon Sep 17 00:00:00 2001 From: Lars Reimann Date: Sun, 16 Apr 2023 22:21:53 +0200 Subject: [PATCH 5/7] test: use data-driven testing --- src/safeds/data/tabular/containers/_table.py | 5 -- .../_table/test_keep_only_columns.py | 57 ++++++++++++------- 2 files changed, 37 insertions(+), 25 deletions(-) diff --git a/src/safeds/data/tabular/containers/_table.py b/src/safeds/data/tabular/containers/_table.py index 9747d5813..668325a63 100644 --- a/src/safeds/data/tabular/containers/_table.py +++ b/src/safeds/data/tabular/containers/_table.py @@ -129,14 +129,9 @@ def from_columns(columns: list[Column]) -> Table: Raises ------ - MissingDataError - If an empty list is given. ColumnLengthMismatchError If any of the column sizes does not match with the others. """ - if len(columns) == 0: - raise MissingDataError("This function requires at least one column.") - dataframe: DataFrame = pd.DataFrame() for column in columns: diff --git a/tests/safeds/data/tabular/containers/_table/test_keep_only_columns.py b/tests/safeds/data/tabular/containers/_table/test_keep_only_columns.py index 0d4b7f5ac..6b376d742 100644 --- a/tests/safeds/data/tabular/containers/_table/test_keep_only_columns.py +++ b/tests/safeds/data/tabular/containers/_table/test_keep_only_columns.py @@ -1,29 +1,46 @@ import pytest + from safeds.data.tabular.containers import Table, Column from safeds.data.tabular.exceptions import UnknownColumnNameError -from tests.helpers import resolve_resource_path - - -def test_keep_only_columns() -> None: - table = Table.from_csv_file(resolve_resource_path("test_table_from_csv_file.csv")) - transformed_table = table.keep_only_columns(["A"]) - assert transformed_table.schema.has_column("A") - assert not transformed_table.schema.has_column("B") - -def test_keep_only_columns_order() -> None: - table = Table.from_csv_file(resolve_resource_path("test_table_from_csv_file.csv")) - transformed_table = table.keep_only_columns(["B", "A"]) - assert transformed_table == Table.from_columns( +class TestKeepOnlyColumns: + @pytest.mark.parametrize( + ("table", "column_names", "expected"), [ - Column("B", [2]), - Column("A", [1]) + ( + Table.from_columns([Column("A", [1]), Column("B", [2])]), + [], + Table.from_columns([]), + ), + ( + Table.from_columns([Column("A", [1]), Column("B", [2])]), + ["A"], + Table.from_columns([Column("A", [1])]), + ), + ( + Table.from_columns([Column("A", [1]), Column("B", [2])]), + ["B"], + Table.from_columns([Column("B", [2])]), + ), + ( + Table.from_columns([Column("A", [1]), Column("B", [2])]), + ["A", "B"], + Table.from_columns([Column("A", [1]), Column("B", [2])]), + ), + # Related to https://github.com/Safe-DS/Stdlib/issues/115 + ( + Table.from_columns([Column("A", [1]), Column("B", [2]), Column("C", [3])]), + ["C", "A"], + Table.from_columns([Column("C", [3]), Column("A", [1])]), + ), ] ) + def test_should_keep_only_listed_columns(self, table: Table, column_names: list[str], expected: Table): + transformed_table = table.keep_only_columns(column_names) + assert transformed_table == expected - -def test_keep_only_columns_warning() -> None: - table = Table.from_csv_file(resolve_resource_path("test_table_from_csv_file.csv")) - with pytest.raises(UnknownColumnNameError): - table.keep_only_columns(["C"]) + def test_raise_if_column_does_no_exist(self): + table = Table.from_columns([Column("A", [1]), Column("B", [2])]) + with pytest.raises(UnknownColumnNameError): + table.keep_only_columns(["C"]) From 2ae3c0d486a6749679aec8758384538da74af117 Mon Sep 17 00:00:00 2001 From: Lars Reimann Date: Sun, 16 Apr 2023 22:24:51 +0200 Subject: [PATCH 6/7] test: remove outdated test --- .../data/tabular/containers/_table/test_from_columns.py | 9 +-------- .../tabular/containers/_table/test_keep_only_columns.py | 4 ++-- 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/tests/safeds/data/tabular/containers/_table/test_from_columns.py b/tests/safeds/data/tabular/containers/_table/test_from_columns.py index 7ce8add2f..a9a0b4e79 100644 --- a/tests/safeds/data/tabular/containers/_table/test_from_columns.py +++ b/tests/safeds/data/tabular/containers/_table/test_from_columns.py @@ -1,8 +1,6 @@ import pandas as pd -import pytest -from safeds.data.tabular.containers import Column, Table -from safeds.data.tabular.exceptions import MissingDataError +from safeds.data.tabular.containers import Column, Table from tests.helpers import resolve_resource_path @@ -15,8 +13,3 @@ def test_from_columns() -> None: table_restored: Table = Table.from_columns(columns_table) assert table_restored == table_expected - - -def test_from_columns_invalid() -> None: - with pytest.raises(MissingDataError): - Table.from_columns([]) diff --git a/tests/safeds/data/tabular/containers/_table/test_keep_only_columns.py b/tests/safeds/data/tabular/containers/_table/test_keep_only_columns.py index 6b376d742..04759a4cd 100644 --- a/tests/safeds/data/tabular/containers/_table/test_keep_only_columns.py +++ b/tests/safeds/data/tabular/containers/_table/test_keep_only_columns.py @@ -36,11 +36,11 @@ class TestKeepOnlyColumns: ), ] ) - def test_should_keep_only_listed_columns(self, table: Table, column_names: list[str], expected: Table): + def test_should_keep_only_listed_columns(self, table: Table, column_names: list[str], expected: Table) -> None: transformed_table = table.keep_only_columns(column_names) assert transformed_table == expected - def test_raise_if_column_does_no_exist(self): + def test_should_raise_if_column_does_no_exist(self) -> None: table = Table.from_columns([Column("A", [1]), Column("B", [2])]) with pytest.raises(UnknownColumnNameError): table.keep_only_columns(["C"]) From e9accc6cb3ee04ecbcd4064d547afa3e3994ccec Mon Sep 17 00:00:00 2001 From: megalinter-bot <129584137+megalinter-bot@users.noreply.github.com> Date: Sun, 16 Apr 2023 20:30:32 +0000 Subject: [PATCH 7/7] style: apply automated linter fixes --- .../data/tabular/containers/_table/test_from_columns.py | 2 +- .../data/tabular/containers/_table/test_keep_only_columns.py | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/safeds/data/tabular/containers/_table/test_from_columns.py b/tests/safeds/data/tabular/containers/_table/test_from_columns.py index a9a0b4e79..34349a350 100644 --- a/tests/safeds/data/tabular/containers/_table/test_from_columns.py +++ b/tests/safeds/data/tabular/containers/_table/test_from_columns.py @@ -1,6 +1,6 @@ import pandas as pd - from safeds.data.tabular.containers import Column, Table + from tests.helpers import resolve_resource_path diff --git a/tests/safeds/data/tabular/containers/_table/test_keep_only_columns.py b/tests/safeds/data/tabular/containers/_table/test_keep_only_columns.py index 04759a4cd..550e8ea7e 100644 --- a/tests/safeds/data/tabular/containers/_table/test_keep_only_columns.py +++ b/tests/safeds/data/tabular/containers/_table/test_keep_only_columns.py @@ -1,6 +1,5 @@ import pytest - -from safeds.data.tabular.containers import Table, Column +from safeds.data.tabular.containers import Column, Table from safeds.data.tabular.exceptions import UnknownColumnNameError @@ -34,7 +33,7 @@ class TestKeepOnlyColumns: ["C", "A"], Table.from_columns([Column("C", [3]), Column("A", [1])]), ), - ] + ], ) def test_should_keep_only_listed_columns(self, table: Table, column_names: list[str], expected: Table) -> None: transformed_table = table.keep_only_columns(column_names)