refactor

refactor

Author: wxtim

changes.d/6874.fix.md

@@ -0,0 +1 @@
+Improved warnings for suspect start and stop cycle points.

cylc/flow/config.py

@@ -433,7 +433,10 @@ def __init__(
         self.process_start_cycle_point()
         self.process_final_cycle_point()
         self.process_stop_cycle_point()
-        self.start_point_checks()
+        if self.start_point:
+            self.cycle_point_warning('start', 'before', 'initial')
+            self.cycle_point_warning('start', 'after', 'final')
+            self.cycle_point_warning('stop', 'before', 'start')
 
         # Parse special task cycle point offsets, and replace family names.
         LOG.debug("Parsing [special tasks]")
@@ -776,29 +779,24 @@ def process_start_cycle_point(self) -> None:
             # Start from the initial point.
             self.start_point = self.initial_point
 
-    def start_point_checks(self):
-        """Check that the start point makes sense.
+    def cycle_point_warning(self, label1, order, label2):
+        """Centralized logic for warning that start, stop, initial and
+        final cycle points are not sensibly ordered.
         """
-        if not self.start_point:
-            return
-        elif self.start_point < self.initial_point:
-            LOG.warning(
-                f"Start cycle point '{self.start_point}' will have no "
-                "effect as it is before the initial cycle "
-                f"point '{self.initial_point}'."
-            )
-        elif self.start_point > self.final_point:
-            LOG.warning(
-                f"Start cycle point '{self.start_point}' will have no "
-                "effect as it is after the final cycle "
-                f"point '{self.final_point}'."
-            )
-        elif self.start_point > self.stop_point:
-            LOG.warning(
-                f"Stop cycle point '{self.stop_point}' will have no "
-                "effect as it is before the start cycle "
-                f"point '{self.start_point}'."
+        point1 = getattr(self, label1 + '_point')
+        point2 = getattr(self, label2 + '_point')
+        if (
+            order == 'before' and point1 < point2
+            or order == 'after' and point1 > point2
+        ):
+            msg = (
+                f"{label1} cycle point '{point1}' will have no effect as"
+                f" it is {order} the {label2} cycle point '{point2}'."
             )
+            # Capitalize() makes the ISO strings look very odd:
+            LOG.warning(msg[0].upper() + msg[1:])
+            return True
+        return False
 
     def process_final_cycle_point(self) -> None:
         """Validate and set the final cycle point from flow.cylc or options.
@@ -880,25 +878,15 @@ def process_stop_cycle_point(self) -> None:
                 self.initial_point,
             ).standardise()
             if (
-                self.final_point is not None
-                and self.stop_point is not None
-                and self.stop_point > self.final_point
-            ):
-                LOG.warning(
-                    f"Stop cycle point '{self.stop_point}' will have no "
-                    "effect as it is after the final cycle "
-                    f"point '{self.final_point}'."
+                self.stop_point is not None 
+                and (
+                    (
+                        self.final_point is not None
+                        and self.cycle_point_warning('stop', 'after', 'final')
+                    )
+                    or self.cycle_point_warning('stop', 'before', 'initial')
                 )
-                self.stop_point = None
-            if (
-                self.stop_point is not None
-                and self.stop_point < self.initial_point
             ):
-                LOG.warning(
-                    f"Stop cycle point '{self.stop_point}' will have no "
-                    "effect as it is before the initial cycle "
-                    f"point '{self.initial_point}'."
-                )
                 self.stop_point = None
             stopcp_str = str(self.stop_point) if self.stop_point else None
             self.cfg['scheduling']['stop after cycle point'] = stopcp_str