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: when using from table to time series feature must be given #572

Merged
Show file tree
Hide file tree
Changes from 13 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
4 changes: 2 additions & 2 deletions src/safeds/data/tabular/containers/_table.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
from __future__ import annotations

import sys
import functools
import io
import sys
import warnings
from pathlib import Path
from typing import TYPE_CHECKING, Any, TypeVar
Expand Down Expand Up @@ -1762,7 +1762,7 @@ def time_columns(self, target_name: str, time_name: str, feature_names: list[str
"""
from ._time_series import TimeSeries

return TimeSeries._from_table_to_time_series(self, target_name, time_name, feature_names)
return TimeSeries._from_table(self, target_name, time_name, feature_names)

def transform_column(self, name: str, transformer: Callable[[Row], Any]) -> Table:
"""
Expand Down
348 changes: 189 additions & 159 deletions src/safeds/data/tabular/containers/_time_series.py

Large diffs are not rendered by default.

5 changes: 3 additions & 2 deletions tests/helpers/_assertions.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@ def assert_that_time_series_are_equal(table1: TimeSeries, table2: TimeSeries) ->
The timeseries to compare the first timeseries to.
"""
assert table1.schema == table2.schema
assert table1.features == table2.features
assert table1.target == table2.target
assert table1._feature_names == table2._feature_names
assert table1._features == table2._features
assert table1._target == table2._target
assert table1.time == table2.time
assert table1 == table2

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
},
target_name="target",
time_name="time",
feature_names=["feature_1"],
feature_names=None,
),
),
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
},
"target",
"time",
["feature_1"],
None,
),
),
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
},
target_name="T",
time_name="time",
feature_names=["A", "B", "C"],
),
Table({"A": [1, 4], "B": [2, 5], "C": [3, 6]}),
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,36 +52,7 @@
"time",
["A", "B", "C"],
ValueError,
r"Column 'A' cannot be both feature and target.",
),
(
Table(
{
"time": [0, 1],
"A": [1, 4],
"B": [2, 5],
"C": [3, 6],
"T": [0, 1],
},
),
"A",
"time",
[],
ValueError,
r"At least one feature column must be specified.",
),
(
Table(
{
"time": [0, 1],
"A": [1, 4],
},
),
"A",
"time",
None,
ValueError,
r"At least one feature column must be specified.",
r"Column 'A' can not be target and feature column.",
),
(
Table(
Expand Down Expand Up @@ -120,8 +91,6 @@
"feature_does_not_exist",
"target_does_not_exist",
"target_and_feature_overlap",
"features_are_empty-explicitly",
"features_are_empty_implicitly",
"time_does_not_exist",
"time_is_also_feature",
],
Expand All @@ -135,7 +104,7 @@ def test_should_raise_error(
error_msg: str,
) -> None:
with pytest.raises(error, match=error_msg):
TimeSeries._from_table_to_time_series(
TimeSeries._from_table(
table,
target_name=target_name,
time_name=time_name,
Expand Down Expand Up @@ -186,7 +155,7 @@ def test_should_raise_error(
),
"T",
"time",
None,
["B"],
),
],
ids=["create_tagged_table", "tagged_table_not_all_columns_are_features", "tagged_table_with_feature_names_as_None"],
Expand All @@ -197,7 +166,7 @@ def test_should_create_a_tagged_table(
time_name: str,
feature_names: list[str] | None,
) -> None:
time_series = TimeSeries._from_table_to_time_series(
time_series = TimeSeries._from_table(
table,
target_name=target_name,
time_name=time_name,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,32 +46,7 @@
"A",
["A", "B", "C"],
ValueError,
r"Column 'A' cannot be both feature and target.",
),
(
{
"time": [0, 1],
"A": [1, 4],
"B": [2, 5],
"C": [3, 6],
"T": [0, 1],
},
"time",
"D",
[],
ValueError,
r"At least one feature column must be specified.",
),
(
{
"time": [0, 1],
"A": [1, 4],
},
"time",
"A",
None,
ValueError,
r"At least one feature column must be specified.",
r"Column 'A' can not be time and feature column.",
),
(
{
Expand Down Expand Up @@ -106,8 +81,6 @@
"feature_does_not_exist",
"target_does_not_exist",
"target_and_feature_overlap",
"features_are_empty-explicitly",
"features_are_empty_implicitly",
"time_column_does_not_exist",
"time_is_also_feature",
],
Expand Down Expand Up @@ -174,11 +147,10 @@ def test_should_create_a_time_series(
) -> None:
time_series = TimeSeries(data, target_name=target_name, time_name=time_name, feature_names=feature_names)
if feature_names is None:
feature_names = list(data.keys())
feature_names.remove(target_name)
feature_names.remove(time_name)
feature_names = []

assert isinstance(time_series, TimeSeries)
assert time_series._features.column_names == feature_names
assert time_series._feature_names == feature_names
assert time_series._target.name == target_name
assert time_series._features == Table(data).keep_only_columns(feature_names)
assert time_series._target == Table(data).get_column(target_name)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
("table", "column_names", "expected"),
[
(
TimeSeries._from_table_to_time_series(
TimeSeries._from_table(
Table(
{
"time": [0, 1, 2],
Expand All @@ -22,7 +22,7 @@
"time",
),
["feat1", "target", "time"],
TimeSeries._from_table_to_time_series(
TimeSeries._from_table(
Table(
{
"time": [0, 1, 2],
Expand All @@ -35,7 +35,7 @@
),
),
(
TimeSeries._from_table_to_time_series(
TimeSeries._from_table(
Table(
{
"time": [0, 1, 2],
Expand All @@ -49,7 +49,7 @@
"time",
),
["feat1", "other", "target", "time"],
TimeSeries._from_table_to_time_series(
TimeSeries._from_table(
Table(
{
"time": [0, 1, 2],
Expand All @@ -63,7 +63,7 @@
),
),
(
TimeSeries._from_table_to_time_series(
TimeSeries._from_table(
Table(
{
"time": [0, 1, 2],
Expand All @@ -77,7 +77,7 @@
"time",
),
["feat1", "target", "time"],
TimeSeries._from_table_to_time_series(
TimeSeries._from_table(
Table(
{
"time": [0, 1, 2],
Expand All @@ -101,7 +101,7 @@ def test_should_return_table(table: TimeSeries, column_names: list[str], expecte
("table", "column_names", "error_msg"),
[
(
TimeSeries._from_table_to_time_series(
TimeSeries._from_table(
Table(
{
"time": [0, 1, 2],
Expand All @@ -119,25 +119,7 @@ def test_should_return_table(table: TimeSeries, column_names: list[str], expecte
r"Illegal schema modification: Must keep the target column.",
),
(
TimeSeries._from_table_to_time_series(
Table(
{
"time": [0, 1, 2],
"feat1": [1, 2, 3],
"feat2": [4, 5, 6],
"other": [3, 5, 7],
"target": [7, 8, 9],
},
),
"target",
"time",
["feat1", "feat2"],
),
["target", "other"],
r"Illegal schema modification: Must keep at least one feature column.",
),
(
TimeSeries._from_table_to_time_series(
TimeSeries._from_table(
Table(
{
"time": [0, 1, 2],
Expand All @@ -155,7 +137,7 @@ def test_should_return_table(table: TimeSeries, column_names: list[str], expecte
r"Illegal schema modification: Must keep the time column.",
),
],
ids=["table_remove_target", "table_remove_all_features", "table_remove_time"],
ids=["table_remove_target", "table_remove_time"],
)
def test_should_raise_illegal_schema_modification(table: TimeSeries, column_names: list[str], error_msg: str) -> None:
with pytest.raises(
Expand Down
Loading