Apply suggestions from code review

- Rename `_bounds_methods.py` -> `_bounds.py`.
- Make methods in `_bounds.py` not private.
- Move private attribute definitions in `Report` class to the end.
- Improve docstring example for `check_exclusion_bounds_overlap`
  method.

Signed-off-by: Sahas Subramanian <[email protected]>

Author: shsms

src/frequenz/sdk/actor/_power_managing/_base_classes.py

@@ -13,7 +13,7 @@
 
 from ... import timeseries
 from ...timeseries import Power
-from ._bounds_methods import _clamp_to_bounds
+from . import _bounds
 
 if typing.TYPE_CHECKING:
     from ...timeseries.battery_pool import PowerMetrics
@@ -50,6 +50,12 @@ class Report:
     target_power: Power | None
     """The currently set power for the batteries."""
 
+    distribution_result: power_distributing.Result | None
+    """The result of the last power distribution.
+
+    This is `None` if no power distribution has been performed yet.
+    """
+
     _inclusion_bounds: timeseries.Bounds[Power] | None
     """The available inclusion bounds for the batteries, for the actor's priority.
 
@@ -67,12 +73,6 @@ class Report:
     priorities.
     """
 
-    distribution_result: power_distributing.Result | None
-    """The result of the last power distribution.
-
-    This is `None` if no power distribution has been performed yet.
-    """
-
     @property
     def bounds(self) -> timeseries.Bounds[Power] | None:
         """The bounds for the batteries.
@@ -140,7 +140,7 @@ def adjust_to_bounds(self, power: Power) -> tuple[Power | None, Power | None]:
         if self._inclusion_bounds is None:
             return None, None
 
-        return _clamp_to_bounds(
+        return _bounds.clamp_to_bounds(
             power,
             self._inclusion_bounds.lower,
             self._inclusion_bounds.upper,

…actor/_power_managing/_bounds_methods.py …enz/sdk/actor/_power_managing/_bounds.pysrc/frequenz/sdk/actor/_power_managing/_bounds_methods.py renamed to src/frequenz/sdk/actor/_power_managing/_bounds.py src/frequenz/sdk/actor/_power_managing/_bounds_methods.py renamed to src/frequenz/sdk/actor/_power_managing/_bounds.py

@@ -1,32 +1,32 @@
 # License: MIT
 # Copyright © 2023 Frequenz Energy-as-a-Service GmbH
 
-"""Methods for checking and clamping bounds and power values to exclusion bounds."""
+"""Utilities for checking and clamping bounds and power values to exclusion bounds."""
 
 from ...timeseries import Bounds, Power
 
 
-def _check_exclusion_bounds_overlap(
+def check_exclusion_bounds_overlap(
     lower_bound: Power,
     upper_bound: Power,
     exclusion_bounds: Bounds[Power] | None,
 ) -> tuple[bool, bool]:
     """Check if the given bounds overlap with the given exclusion bounds.
 
-    When only the upper bound overlaps with exclusion bounds, the usable range is
-    between the lower bound and the lower exclusion bound, like below.
+    Example:
 
-      ===lb+++++++ex----ub-------ex===
+                       lower                        upper
+                          .----- exclusion zone -----.
+        -----|✓✓✓✓✓✓✓✓✓✓✓✓|xxxxxxxxxxxxxxx|----------|----
+             `-- usable --'-- exclusion --´
+             |                 overlap    |
+             |                            |
+           lower                        upper
+           bound                        bound
+                              (inside the exclusion zone)
 
-    When only the lower bound overlaps with exclusion bounds, the usable range is
-    between the upper exclusion bound and the upper bound.
-
-      ===ex------lb------ex++++++ub===
-
-    Both bounds overlapping with exclusion bounds (or given bounds are fully contained
-    within exclusion bounds).  In this case, there is no usable range.
-
-      ===ex------lb------ub------ex===
+        Resulting in `(False, True)` because only the upper bound is inside the
+        exclusion zone.
 
     Args:
         lower_bound: The lower bound to check.
@@ -52,7 +52,7 @@ def _check_exclusion_bounds_overlap(
     return bounded_lower, bounded_upper
 
 
-def _adjust_exclusion_bounds(
+def adjust_exclusion_bounds(
     lower_bound: Power,
     upper_bound: Power,
     exclusion_bounds: Bounds[Power] | None,
@@ -75,7 +75,7 @@ def _adjust_exclusion_bounds(
     #
     # And if the given bounds overlap with the exclusion bounds on one side, then clamp
     # the given bounds on that side.
-    match _check_exclusion_bounds_overlap(lower_bound, upper_bound, exclusion_bounds):
+    match check_exclusion_bounds_overlap(lower_bound, upper_bound, exclusion_bounds):
         case (True, True):
             return Power.zero(), Power.zero()
         case (False, True):
@@ -87,7 +87,7 @@ def _adjust_exclusion_bounds(
 
 # Just 20 lines of code in this function, but unfortunately 8 of those are return
 # statements, and that's too many for pylint.
-def _clamp_to_bounds(  # pylint: disable=too-many-return-statements
+def clamp_to_bounds(  # pylint: disable=too-many-return-statements
     value: Power,
     lower_bound: Power,
     upper_bound: Power,
@@ -116,7 +116,7 @@ def _clamp_to_bounds(  # pylint: disable=too-many-return-statements
     # given power is in that overlap region, clamp it to the exclusion bounds on that
     # side.
     if exclusion_bounds is not None:
-        match _check_exclusion_bounds_overlap(
+        match check_exclusion_bounds_overlap(
             lower_bound, upper_bound, exclusion_bounds
         ):
             case (True, True):

src/frequenz/sdk/actor/_power_managing/_matryoshka.py

@@ -26,12 +26,8 @@
 
 from ... import timeseries
 from ...timeseries import Power
+from . import _bounds
 from ._base_classes import BaseAlgorithm, Proposal, Report
-from ._bounds_methods import (
-    _adjust_exclusion_bounds,
-    _check_exclusion_bounds_overlap,
-    _clamp_to_bounds,
-)
 from ._sorted_set import SortedSet
 
 if typing.TYPE_CHECKING:
@@ -89,7 +85,7 @@ def _calc_target_power(
             if upper_bound < lower_bound:
                 break
             if next_proposal.preferred_power:
-                match _clamp_to_bounds(
+                match _bounds.clamp_to_bounds(
                     next_proposal.preferred_power,
                     lower_bound,
                     upper_bound,
@@ -111,14 +107,14 @@ def _calc_target_power(
             # If the bounds from the current proposal are fully within the exclusion
             # bounds, then don't use them to narrow the bounds further. This allows
             # subsequent proposals to not be blocked by the current proposal.
-            match _check_exclusion_bounds_overlap(
+            match _bounds.check_exclusion_bounds_overlap(
                 proposal_lower, proposal_upper, exclusion_bounds
             ):
                 case (True, True):
                     continue
             lower_bound = max(lower_bound, proposal_lower)
             upper_bound = min(upper_bound, proposal_upper)
-            lower_bound, upper_bound = _adjust_exclusion_bounds(
+            lower_bound, upper_bound = _bounds.adjust_exclusion_bounds(
                 lower_bound, upper_bound, exclusion_bounds
             )
 
@@ -248,15 +244,15 @@ def get_status(
                 break
             proposal_lower = next_proposal.bounds.lower or lower_bound
             proposal_upper = next_proposal.bounds.upper or upper_bound
-            match _check_exclusion_bounds_overlap(
+            match _bounds.check_exclusion_bounds_overlap(
                 proposal_lower, proposal_upper, exclusion_bounds
             ):
                 case (True, True):
                     continue
             calc_lower_bound = max(lower_bound, proposal_lower)
             calc_upper_bound = min(upper_bound, proposal_upper)
             if calc_lower_bound <= calc_upper_bound:
-                lower_bound, upper_bound = _adjust_exclusion_bounds(
+                lower_bound, upper_bound = _bounds.adjust_exclusion_bounds(
                     calc_lower_bound, calc_upper_bound, exclusion_bounds
                 )
             else: