Review/897/refactoring (#908)

* refactor: fix spelling Signed-off-by: F.N. Claessen <felix@seita.nl> * style: missing type annotation Signed-off-by: F.N. Claessen <felix@seita.nl> * refactor: method and variable renaming Signed-off-by: F.N. Claessen <felix@seita.nl> * refactor: add test case explanations Signed-off-by: F.N. Claessen <felix@seita.nl> --------- Signed-off-by: F.N. Claessen <felix@seita.nl>
FlexMeasures · Nov 24, 2023 · f39c932 · f39c932
1 parent a9aaf21
commit f39c932
Show file tree

Hide file tree

Showing 3 changed files with 80 additions and 61 deletions.
diff --git a/flexmeasures/data/models/planning/storage.py b/flexmeasures/data/models/planning/storage.py
@@ -19,7 +19,7 @@
     initialize_df,
     get_power_values,
     fallback_charging_policy,
-    get_continous_series_sensor_or_quantity,
+    get_continuous_series_sensor_or_quantity,
 )
 from flexmeasures.data.models.planning.exceptions import InfeasibleProblemException
 from flexmeasures.data.schemas.scheduling.storage import StorageFlexModelSchema
@@ -207,10 +207,10 @@ def _prepare(self, skip_validation: bool = False) -> tuple:  # noqa: C901
         else:
             device_constraints[0]["derivative min"] = (
                 -1
-            ) * get_continous_series_sensor_or_quantity(
+            ) * get_continuous_series_sensor_or_quantity(
                 quantity_or_sensor=production_capacity,
                 actuator=sensor,
-                target_unit=sensor.unit,
+                unit=sensor.unit,
                 query_window=(start, end),
                 resolution=resolution,
                 beliefs_before=belief_time,
@@ -223,10 +223,10 @@ def _prepare(self, skip_validation: bool = False) -> tuple:  # noqa: C901
         else:
             device_constraints[0][
                 "derivative max"
-            ] = get_continous_series_sensor_or_quantity(
+            ] = get_continuous_series_sensor_or_quantity(
                 quantity_or_sensor=consumption_capacity,
                 actuator=sensor,
-                target_unit=sensor.unit,
+                unit=sensor.unit,
                 query_window=(start, end),
                 resolution=resolution,
                 beliefs_before=belief_time,

diff --git a/flexmeasures/data/models/planning/tests/test_solver.py b/flexmeasures/data/models/planning/tests/test_solver.py
@@ -1260,16 +1260,18 @@ def set_if_not_none(dictionary, key, value):
             False,
             None,
             None,
-            [-8] * 24 * 4,
-            [0.5] * 24 * 4,
-        ),  # default to production_capacity and consumption_capacity sensor attribute
+            [-8] * 24 * 4,  # from the power sensor attribute 'production_capacity'
+            [0.5] * 24 * 4,  # from the power sensor attribute 'consumption_capacity'
+        ),
         (
             "Test battery with dynamic power capacity",
             True,
             False,
             None,
             None,
+            # from the flex model field 'production-capacity' (a sensor)
             [-0.2] * 4 * 4 + [-0.3] * 4 * 4 + [-8] * 16 * 4,
+            # from the power sensor attribute 'consumption_capacity'
             [0.5] * 24 * 4,
         ),
         (
@@ -1278,7 +1280,9 @@ def set_if_not_none(dictionary, key, value):
             True,
             None,
             None,
+            # from the power sensor attribute 'consumption_capacity'
             [-8] * 24 * 4,
+            # from the flex model field 'consumption-capacity' (a sensor)
             [0.25] * 4 * 4 + [0.15] * 4 * 4 + [0.5] * 16 * 4,
         ),
         (
@@ -1287,7 +1291,9 @@ def set_if_not_none(dictionary, key, value):
             False,
             "100 kW",
             "200 kW",
+            # from the flex model field 'production-capacity' (a quantity)
             [-0.1] * 24 * 4,
+            # from the flex model field 'consumption-capacity' (a quantity)
             [0.2] * 24 * 4,
         ),
         (
@@ -1296,7 +1302,9 @@ def set_if_not_none(dictionary, key, value):
             False,
             "1 MW",
             "2 MW",
+            # from the flex model field 'production-capacity' (a quantity)
             [-1] * 24 * 4,
+            # from the power sensor attribute 'consumption_capacity' (a quantity)
             [0.5] * 24 * 4,
         ),
         (
@@ -1305,17 +1313,31 @@ def set_if_not_none(dictionary, key, value):
             False,
             None,
             None,
+            # from the asset attribute 'capacity_in_mw'
             [-2] * 24 * 4,
+            # from the asset attribute 'capacity_in_mw'
             [2] * 24 * 4,
-        ),  # defaults to capacity_in_mw
-        ("Test battery", False, False, "10 kW", None, [-0.01] * 24 * 4, [2] * 24 * 4),
+        ),
+        (
+            "Test battery",
+            False,
+            False,
+            "10 kW",
+            None,
+            # from the flex model field 'production-capacity' (a quantity)
+            [-0.01] * 24 * 4,
+            # from the asset attribute 'capacity_in_mw'
+            [2] * 24 * 4,
+        ),
         (
             "Test battery",
             False,
             False,
             "10 kW",
             "100 kW",
+            # from the flex model field 'production-capacity' (a quantity)
             [-0.01] * 24 * 4,
+            # from the flex model field 'consumption-capacity' (a quantity)
             [0.1] * 24 * 4,
         ),
     ],

diff --git a/flexmeasures/data/models/planning/utils.py b/flexmeasures/data/models/planning/utils.py
@@ -294,64 +294,61 @@ def idle_after_reaching_target(
     return schedule
 
 
-def get_quantity_attribute(
-    actuator: Asset | Sensor,
+def get_quantity_from_attribute(
+    entity: Asset | Sensor,
     attribute: str,
-    target_unit: str | ur.Quantity,
+    unit: str | ur.Quantity,
     default: float = np.nan,
-):
+) -> float:
+    """Get the value (in the given unit) of a quantity stored as an entity attribute.
+
+    :param entity:      The entity (sensor or asset) containing the attribute to retrieve the value from.
+    :param attribute:   The attribute name to extract the value from.
+    :param unit:        The unit in which the value should be returned.
+    :param default:     The fallback value if the attribute is missing or conversion fails (defaults to np.nan).
+    :return:            The retrieved value or the provided default.
     """
-    Retrieves a quantity value an actuator attribute or returns a provided default.
-
-
-    :param actuator: The Asset or Sensor containing the attribute to retrieve the value from.
-    :param attribute: The attribute name to extract the value from.
-    :param target_unit: The unit in which the value should be returned.
-    :param default: The fallback value if the attribute is missing or conversion fails. Defaults to np.nan.
-    :return: The value retrieved or the provided default if not found or conversion fails.
-    """
-    # get the default value from the actuator attribute. if missing, use default_value
-    value: str | float | int | None = actuator.get_attribute(attribute, default)
+    # get the default value from the entity attribute. if missing, use default_value
+    value: str | float | int | None = entity.get_attribute(attribute, default)
 
     # if it's a string, let's try to convert it to a unit
     if isinstance(value, str):
         try:
             value = ur.Quantity(value)
 
             # convert default value to the target units
-            value = value.to(target_unit).magnitude
+            value = value.to(unit).magnitude
 
         except (UndefinedUnitError, DimensionalityError, ValueError, AssertionError):
-            current_app.logger.warning(f"Couldn't convert {value} to `{target_unit}`")
+            current_app.logger.warning(f"Couldn't convert {value} to `{unit}`")
             return default
 
     return value
 
 
-def get_series_from_sensor_or_quantity(
+def get_series_from_quantity_or_sensor(
     quantity_or_sensor: Sensor | ur.Quantity | None,
-    target_unit: ur.Quantity | str,
+    unit: ur.Quantity | str,
     query_window: tuple[datetime, datetime],
     resolution: timedelta,
     beliefs_before: datetime | None = None,
 ) -> pd.Series:
     """
-    Get a time series from a quantity or Sensor defined on a time window.
-
-    :param quantity_or_sensor: input sensor or pint Quantity
-    :param actuator: sensor of an actuator. This could be a power capacity sensor, efficiency, etc.
-    :param target_unit: unit of the output data.
-    :param query_window: tuple representing the start and end of the requested data
-    :param resolution: time resolution of the requested data
-    :param beliefs_before: datetime used to indicate we are interested in the state of knowledge at that time, defaults to None
-    :return: pandas Series with the requested time series data
+    Get a time series given a quantity or sensor defined on a time window.
+
+    :param quantity_or_sensor:  pint Quantity or timely-beliefs Sensor, measuring e.g. power capacity or efficiency
+    :param unit:                unit of the output data.
+    :param query_window:        tuple representing the start and end of the requested data
+    :param resolution:          time resolution of the requested data
+    :param beliefs_before:      optional datetime used to indicate we are interested in the state of knowledge at that time
+    :return:                    pandas Series with the requested time series data
     """
 
     start, end = query_window
     time_series = initialize_series(np.nan, start=start, end=end, resolution=resolution)
 
     if isinstance(quantity_or_sensor, ur.Quantity):
-        time_series[:] = quantity_or_sensor.to(target_unit).magnitude
+        time_series[:] = quantity_or_sensor.to(unit).magnitude
     elif isinstance(quantity_or_sensor, Sensor):
         bdf: tb.BeliefsDataFrame = TimedBelief.search(
             quantity_or_sensor,
@@ -364,16 +361,16 @@ def get_series_from_sensor_or_quantity(
         )
         df = simplify_index(bdf).reindex(time_series.index)
         time_series[:] = df.values.squeeze()  # drop unused dimension (N,1) -> (N)
-        time_series = convert_units(time_series, quantity_or_sensor.unit, target_unit)
+        time_series = convert_units(time_series, quantity_or_sensor.unit, unit)
         time_series = cast(pd.Series, time_series)
 
     return time_series
 
 
-def get_continous_series_sensor_or_quantity(
+def get_continuous_series_sensor_or_quantity(
     quantity_or_sensor: Sensor | ur.Quantity | None,
     actuator: Sensor | Asset,
-    target_unit: ur.Quantity | str,
+    unit: ur.Quantity | str,
     query_window: tuple[datetime, datetime],
     resolution: timedelta,
     default_value_attribute: str | None = None,
@@ -390,35 +387,35 @@ def get_continous_series_sensor_or_quantity(
         - 'upper' clips missing values to the upper bound of the default value.
         - 'lower' clips missing values to the lower bound of the default value.
 
-    :param quantity_or_sensor: The sensor or quantity data source.
-    :param actuator: The actuator associated with the data.
-    :param target_unit: The desired unit for the data.
-    :param query_window: The time window (start, end) to query the data.
-    :param resolution: The resolution or time interval for the data.
+    :param quantity_or_sensor:      The quantity or sensor containing the data.
+    :param actuator:                The actuator from which relevant defaults are retrieved.
+    :param unit:                    The desired unit of the data.
+    :param query_window:            The time window (start, end) to query the data.
+    :param resolution:              The resolution or time interval for the data.
     :param default_value_attribute: Attribute for a default value if data is missing.
-    :param default_value: Default value if no attribute or data found.
-    :param beliefs_before: Timestamp for prior beliefs or knowledge.
-    :param method: Method for handling missing data: 'replace', 'upper', 'lower', 'max', or 'min'.
-    :returns: time series data with missing values handled based on the chosen method.
-    :raises: NotImplementedError: If an unsupported method is provided.
+    :param default_value:           Default value if no attribute or data found.
+    :param beliefs_before:          Timestamp for prior beliefs or knowledge.
+    :param method:                  Method for handling missing data: 'replace', 'upper', 'lower', 'max', or 'min'.
+    :returns:                       time series data with missing values handled based on the chosen method.
+    :raises: NotImplementedError:   If an unsupported method is provided.
     """
 
     _default_value = np.nan
 
     if default_value_attribute is not None:
-        _default_value = get_quantity_attribute(
-            actuator=actuator,
+        _default_value = get_quantity_from_attribute(
+            entity=actuator,
             attribute=default_value_attribute,
-            target_unit=target_unit,
+            unit=unit,
             default=default_value,
         )
 
-    time_series = get_series_from_sensor_or_quantity(
-        quantity_or_sensor,
-        target_unit,
-        query_window,
-        resolution,
-        beliefs_before,
+    time_series = get_series_from_quantity_or_sensor(
+        quantity_or_sensor=quantity_or_sensor,
+        unit=unit,
+        query_window=query_window,
+        resolution=resolution,
+        beliefs_before=beliefs_before,
     )
 
     if method == "replace":