From 6b29bf4afd213842051f825f6afb165bb64c44e3 Mon Sep 17 00:00:00 2001 From: j-atkins <106238905+j-atkins@users.noreply.github.com> Date: Wed, 30 Sep 2026 14:28:57 +0200 Subject: [PATCH 1/2] fix mismatch between wps_in_use indexing and full waypoints (including non-active Ports); add another test param to ensure getting the correct waypoint --- src/virtualship/models/expedition.py | 7 ++++++- tests/expedition/test_expedition.py | 19 +++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/virtualship/models/expedition.py b/src/virtualship/models/expedition.py index 12ac5a9b..c6e6ff7a 100644 --- a/src/virtualship/models/expedition.py +++ b/src/virtualship/models/expedition.py @@ -234,7 +234,12 @@ def verify( # check that ship will arrive on time at each waypoint (in case no unexpected event happen) time = wps_in_use[0].time - for wp_i, (wp, wp_next) in enumerate(itertools.pairwise(wps_in_use)): + # offset from wps_in_use indices to self.waypoints indices (a placeholder departure port is excluded from wps_in_use) + wps_in_use_offset = 0 if self.departure_port.is_in_use else 1 + + for wp_i, (wp, wp_next) in enumerate( + itertools.pairwise(wps_in_use), start=wps_in_use_offset + ): stationkeeping_time = ( wp.stationkeeping_time(instruments_config) if isinstance(wp, Waypoint) diff --git a/tests/expedition/test_expedition.py b/tests/expedition/test_expedition.py index 418c7197..3a28eddb 100644 --- a/tests/expedition/test_expedition.py +++ b/tests/expedition/test_expedition.py @@ -235,6 +235,25 @@ def test_verify_on_land(base_expedition): r"Waypoint planning is not valid: would arrive too late at waypoint 2\.", id="NotEnoughTime", ), + pytest.param( + [ + Port(location=Location(None, None), time=None), + Waypoint( + location=Location(0, 0), + time=datetime(2022, 1, 1, 1, 0, 0), + instrument=[], + ), + Waypoint( + location=Location(1, 0), + time=datetime(2022, 1, 1, 1, 1, 0), + instrument=[], + ), + Port(location=Location(1, 0), time=datetime(2022, 1, 2, 0, 0, 0)), + ], + ScheduleError, + r"Waypoint planning is not valid: would arrive too late at waypoint 2\.", + id="NotEnoughTimePlaceholderDeparturePort", + ), ], ) def test_verify_schedule_errors(base_expedition, waypoints: list, error, match) -> None: From ea6c5b6edb744cd888a1c476df376f0787b91c79 Mon Sep 17 00:00:00 2001 From: j-atkins <106238905+j-atkins@users.noreply.github.com> Date: Wed, 30 Sep 2026 14:29:25 +0200 Subject: [PATCH 2/2] keep indices in time with full waypoints list so that the correct waypoints are named in the out-of-order waypoint times error --- src/virtualship/models/expedition.py | 23 ++++++++++++++++------- tests/expedition/test_expedition.py | 12 +++++++++++- 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/src/virtualship/models/expedition.py b/src/virtualship/models/expedition.py index c6e6ff7a..60e3c4fb 100644 --- a/src/virtualship/models/expedition.py +++ b/src/virtualship/models/expedition.py @@ -187,15 +187,24 @@ def verify( raise ScheduleError(f"{wp_str} must have a specified time.") # check waypoint times are in ascending order - timed_waypoints = [wp for wp in self.waypoints if wp.time is not None] - checks = [ - next.time >= cur.time for cur, next in itertools.pairwise(timed_waypoints) + # (indices kept relative to self.waypoints, so they can be converted to public waypoint numbers) + timed_waypoints = [ + (wp_i, wp) for wp_i, wp in enumerate(self.waypoints) if wp.time is not None ] - if not all(checks): - invalid_i = [i for i, c in enumerate(checks) if c] - public_wps = [_get_public_wp(i, self.waypoints) for i in invalid_i] + invalid_i = [ + next_i + for (_, cur), (next_i, next) in itertools.pairwise(timed_waypoints) + if next.time < cur.time + ] + if invalid_i: + invalid_labels = [ + "Port of Arrival" + if _get_public_wp(i, self.waypoints) is None + else f"#{_get_public_wp(i, self.waypoints)}" + for i in invalid_i + ] raise ScheduleError( - f"Waypoint(s) {', '.join(f'#{i}' for i in public_wps)}: each waypoint should be timed after all previous waypoints", + f"Waypoint(s) {', '.join(invalid_labels)}: each waypoint should be timed after all previous waypoints", ) # check if all non-port waypoints are in water using bathymetry data diff --git a/tests/expedition/test_expedition.py b/tests/expedition/test_expedition.py index 3a28eddb..b40f6dd0 100644 --- a/tests/expedition/test_expedition.py +++ b/tests/expedition/test_expedition.py @@ -213,9 +213,19 @@ def test_verify_on_land(base_expedition): Port(location=Location(1, 0), time=datetime(2022, 1, 3, 0, 0, 0)), ], ScheduleError, - r"Waypoint\(s\).*?: each waypoint should be timed after all previous waypoints", + r"Waypoint\(s\) #3: each waypoint should be timed after all previous waypoints", id="SequentialWaypoints", ), + pytest.param( + [ + Port(location=Location(0, 0), time=datetime(2022, 1, 1, 0, 0, 0)), + Waypoint(location=Location(0, 0), time=datetime(2022, 1, 2, 0, 0, 0)), + Port(location=Location(1, 0), time=datetime(2022, 1, 1, 12, 0, 0)), + ], + ScheduleError, + r"Waypoint\(s\) Port of Arrival: each waypoint should be timed after all previous waypoints", + id="SequentialWaypointsArrivalPort", + ), pytest.param( [ Port(location=Location(0, 0), time=datetime(2022, 1, 1, 0, 0, 0)),