Skip to content

Commit

Permalink
Apply suggestions from code review
Browse files Browse the repository at this point in the history 
Co-authored-by: Alexander <47296670+Marsmaennchen221@users.noreply.github.com>
  • Loading branch information
@Gerhardsa0 @Marsmaennchen221
Gerhardsa0 and Marsmaennchen221 committed Jan 24, 2024
1 parent 8982880 commit 262b9e6
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 58 deletions.
6 changes: 2 additions & 4 deletions src/safeds/data/tabular/containers/_table.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1730,12 +1730,12 @@ def time_columns(self, target_name: str, time_name: str, feature_names: list[str
time_name : str
Name of the time column.
feature_names : list[str] | None
Names of the feature columns. If None, all columns except the target column are used.
Names of the feature columns. If None, all columns except the target and time columns are used.
Returns
-------
time_series : TimeSeries
A new tagged table with the given target and feature names.
A new time series with the given target, time and feature names.
Raises
------
Expand All @@ -1750,8 +1750,6 @@ def time_columns(self, target_name: str, time_name: str, feature_names: list[str
>>> table = Table.from_dict({"time": ["01.01", "01.02", "01.03"], "price": [1.10, 1.19, 1.79], "amount_bought": [74, 72, 51]})
>>> tagged_table = table.time_columns(target_name="amount_bought",time_name = "time", feature_names=["price"])
"""
from ._time_series import TimeSeries

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

def transform_column(self, name: str, transformer: Callable[[Row], Any]) -> Table:
Expand Down
2 changes: 1 addition & 1 deletion src/safeds/data/tabular/containers/_tagged_table.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,7 @@ def filter_rows(self, query: Callable[[Row], bool]) -> TaggedTable:
Returns
-------
result : TaggedTable
A table containing only the rows to match the query.
A new tagged table containing only the rows to match the query.
"""
return TaggedTable._from_table(
super().filter_rows(query),
Expand Down
84 changes: 31 additions & 53 deletions src/safeds/data/tabular/containers/_time_series.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ def _from_tagged_table(
The tagged table.
time_name: str
Name of the time column.
Retruns
Returns
-------
time_series : TimeSeries
the created time series
Expand Down Expand Up @@ -94,7 +94,6 @@ def _from_tagged_table(
return result

@staticmethod
# idk if this method should exist, because we cant use it like the Method in TaggedTable, but the Method is static and this seem to doesnt get checked by the megalinter.
def _from_table_to_time_series(
table: Table,
target_name: str,
Expand All @@ -112,16 +111,16 @@ def _from_table_to_time_series(
time_name: str
Name of the date column.
feature_names : list[str] | None
Names of the feature columns. If None, all columns except the target column are used.
Retruns
Names of the feature columns. If None, all columns except the target and time columns are used.
Returns
-------
time_series : TimeSeries
the created time series
Raises
------
UnknownColumnNameError
If target_name matches none of the column names.
If target_name or time_name matches none of the column names.
Value Error
If no feature columns are specified
Expand Down Expand Up @@ -153,7 +152,7 @@ def __init__(
feature_names: list[str] | None = None,
):
"""
Create a tagged table from a mapping of column names to their values.
Create a time series from a mapping of column names to their values.
Parameters
----------
Expand All @@ -164,7 +163,7 @@ def __init__(
time_name : str
Name of the time column
feature_names : list[str] | None
Names of the feature columns. If None, all columns except the target column are used.
Names of the feature columns. If None, all columns except the target and time columns are used.
Raises
------
Expand Down Expand Up @@ -214,35 +213,14 @@ def time(self) -> Column:
"""
return self._time

# ------------------------------------------------------------------------------------------------------------------
# Specific methods from time series class:
# ------------------------------------------------------------------------------------------------------------------

# def _create_moving_average(self, window_size: int) -> Column:
# calculate the windows and the moving average of a timeserties, this function for now only has intern use
# some unexpected behavior while casting to series agains, because it addds NAN values
# return Column._from_pandas_series(pd.Series(self.target._data.rolling(window_size).mean()))
# pass
# def plot_moving_average(self, window_size: int) -> Image:
# this method should plot the moving average of a time series
# pass

# def show_time_series(self) -> Image:
# this method should plot the time series
# pass

# def decompose_time_series(self) -> Image:
# this Method should show a plot of a decomposed time series
# pass

# ------------------------------------------------------------------------------------------------------------------
# Overriden methods from TaggedTable class:
# ------------------------------------------------------------------------------------------------------------------
def _as_table(self: TimeSeries) -> Table:
"""
Return a new `Table` with the tagging removed.
The original table is not modified.
The original time series is not modified.
Parameters
----------
Expand All @@ -252,7 +230,7 @@ def _as_table(self: TimeSeries) -> Table:
Returns
-------
table: Table
The table as an untagged Table, i.e. without the information about which columns are features, target or time.
The time series as an untagged Table, i.e. without the information about which columns are features, target or time.
"""
return Table.from_columns(super().to_columns())
Expand All @@ -261,7 +239,7 @@ def add_column(self, column: Column) -> TimeSeries:
"""
Return a new `TimeSeries` with the provided column attached at the end, as neither target nor feature column.
The original table is not modified.
The original time series is not modified.
Parameters
----------
Expand All @@ -271,7 +249,7 @@ def add_column(self, column: Column) -> TimeSeries:
Returns
-------
result : TimeSeries
The table with the column attached as neither target nor feature column.
The time series with the column attached as neither target nor feature column.
Raises
------
Expand Down Expand Up @@ -299,7 +277,7 @@ def add_column_as_feature(self, column: Column) -> TimeSeries:
Returns
-------
result : TimeSeries
The table with the attached feature column.
The time series with the attached feature column.
Raises
------
Expand Down Expand Up @@ -345,7 +323,7 @@ def add_columns(self, columns: list[Column] | Table) -> TimeSeries:
"""
Return a new `TimeSeries` with multiple added columns, as neither target nor feature columns.
The original table is not modified.
The original time series is not modified.
Parameters
----------
Expand All @@ -360,9 +338,9 @@ def add_columns(self, columns: list[Column] | Table) -> TimeSeries:
Raises
------
DuplicateColumnNameError
If at least one column name from the provided column list already exists in the table.
If at least one column name from the provided column list already exists in the time series.
ColumnSizeError
If at least one of the column sizes from the provided column list does not match the table.
If at least one of the column sizes from the provided column list does not match the time series.
"""
return TimeSeries._from_tagged_table(
super().add_columns(columns),
Expand All @@ -373,7 +351,7 @@ def add_row(self, row: Row) -> TimeSeries:
"""
Return a new `TimeSeries` with an extra Row attached.
The original table is not modified.
The original time series is not modified.
Parameters
----------
Expand All @@ -388,15 +366,15 @@ def add_row(self, row: Row) -> TimeSeries:
Raises
------
UnknownColumnNameError
If the row has different column names than the table.
If the row has different column names than the time series.
"""
return TimeSeries._from_tagged_table(super().add_row(row), time_name=self.time.name)

def add_rows(self, rows: list[Row] | Table) -> TimeSeries:
"""
Return a new `TimeSeries` with multiple extra Rows attached.
The original table is not modified.
The original time series is not modified.
Parameters
----------
Expand All @@ -406,12 +384,12 @@ def add_rows(self, rows: list[Row] | Table) -> TimeSeries:
Returns
-------
result : TimeSeries
A new time series which combines the original table and the given rows.
A new time series which combines the original time series and the given rows.
Raises
------
UnknownColumnNameError
If at least one of the rows have different column names than the table.
If at least one of the rows have different column names than the time series.
"""
return TimeSeries._from_tagged_table(super().add_rows(rows), time_name=self.time.name)

Expand Down Expand Up @@ -440,7 +418,7 @@ def keep_only_columns(self, column_names: list[str]) -> TimeSeries:
"""
Return a new `TimeSeries` with only the given column(s).
The original table is not modified.
The original time series is not modified.
Parameters
----------
Expand All @@ -457,7 +435,7 @@ def keep_only_columns(self, column_names: list[str]) -> TimeSeries:
UnknownColumnNameError
If any of the given columns does not exist.
IllegalSchemaModificationError
If none of the given columns is the target column or any of the feature columns.
If none of the given columns is the target or time column or any of the feature columns.
"""
if self.target.name not in column_names:
raise IllegalSchemaModificationError("Must keep the target column.")
Expand All @@ -479,9 +457,9 @@ def keep_only_columns(self, column_names: list[str]) -> TimeSeries:

def remove_columns(self, column_names: list[str]) -> TimeSeries:
"""
Return a new `TimeSeries` with the given column(s) removed from the table.
Return a new `TimeSeries` with the given column(s) removed from the time series.
The original table is not modified.
The original time series is not modified.
Parameters
----------
Expand Down Expand Up @@ -526,7 +504,7 @@ def remove_columns_with_missing_values(self) -> TimeSeries:
"""
Return a new `TimeSeries` with every column that misses values removed.
The original table is not modified.
The original time series is not modified.
Returns
-------
Expand Down Expand Up @@ -561,7 +539,7 @@ def remove_columns_with_non_numerical_values(self) -> TimeSeries:
"""
Return a new `TimeSeries` with every column that contains non-numerical values removed.
The original table is not modified.
The original time series is not modified.
Returns
-------
Expand Down Expand Up @@ -665,9 +643,9 @@ def rename_column(self, old_name: str, new_name: str) -> TimeSeries:
Parameters
----------
old_name : str
The old name of the target column.
The old name of the column.
new_name : str
The new name of the target column.
The new name of the column.
Returns
-------
Expand Down Expand Up @@ -705,7 +683,7 @@ def replace_column(self, old_column_name: str, new_columns: list[Column]) -> Tim
becomes the new target or time column. If the column to be replaced is a feature column, the new columns that replace it
all become feature columns.
The order of columns is kept. The original table is not modified.
The order of columns is kept. The original time series is not modified.
Parameters
----------
Expand All @@ -717,7 +695,7 @@ def replace_column(self, old_column_name: str, new_columns: list[Column]) -> Tim
Returns
-------
result : TimeSeries
A time series with the old column replaced by the new column.
A time series with the old column replaced by the new columns.
Raises
------
Expand All @@ -728,7 +706,7 @@ def replace_column(self, old_column_name: str, new_columns: list[Column]) -> Tim
ColumnSizeError
If the size of the column does not match the amount of rows.
IllegalSchemaModificationError
If the target column would be removed or replaced by more than one column.
If the target or time column would be removed or replaced by more than one column.
"""
if old_column_name == self.time.name:
if len(new_columns) != 1:
Expand Down Expand Up @@ -830,7 +808,7 @@ def sort_columns(
If no comparator is given, the columns will be sorted alphabetically by their name.
The original table is not modified.
The original time series is not modified.
Parameters
----------
Expand Down

0 comments on commit 262b9e6

Please sign in to comment.