diff --git a/docs/user-guide/tutorials/working_with_expedition_yaml.md b/docs/user-guide/tutorials/working_with_expedition_yaml.md index 8e5d078d..56d37506 100644 --- a/docs/user-guide/tutorials/working_with_expedition_yaml.md +++ b/docs/user-guide/tutorials/working_with_expedition_yaml.md @@ -117,7 +117,15 @@ You can do multiple `DRIFTER` deployments at the same waypoint by adding multipl - **Time (`time`)**: The scheduled time for reaching the waypoint, specifically in YYYY-MM-DD HH:MM:SS format. -- **Sea depth (`sea_depth_meter`)**: The estimated depth of the water column at the waypoint. This value matters because it determines how a long a deployment takes if there is a depth dependence, e.g. for a CTD that has to be lowered and raised. If you leave it as `null`, VirtualShip doesn't know how deep the water is and assumes the CTD descends all the way to its configured `max_depth_meter`. That is the slowest possible cast, so your schedule is verified against a worst case rather than an optimistic one in the simulations. +- **Sea depth (`sea_depth_meter`, optional)**: The estimated water depth at the waypoint, as a negative number of metres (e.g. `-1500.0`). It is filled in automatically by `virtualship init --from-mfp`, and can otherwise be added by hand. It is only used to estimate how long depth-dependent deployments (e.g. CTD casts) hold the ship at the waypoint. If it is left empty, these are assumed to reach the instrument's configured maximum depth, so time taken at shallow-water waypoints could be overestimated if the configured max depth exceeds the actual depth. The simulated measurements are unaffected either way because the instruments will find and use the actual depth at simulation time. + +```{warning} +Setting `sea_depth_meter` to a positive number will be rejected. Depths throughout `expedition.yaml` are written as negative numbers, measured downwards from the sea surface. This is the opposite sign convention to the MFP export, which reports sea depth as a positive number. +``` + +##### Comments + +You may notice comments such as `# Port of Departure`, `# Waypoint 1`, `# Waypoint 2`, etc. next to each entry in the `waypoints` list, as in the example above. These are added automatically whenever `virtualship init` or `virtualship plan` writes the file, purely to improve readability. They are not required when editing the file by hand and won't be automatically added/renumbered when doing so. #### 2. `instruments_config` diff --git a/src/virtualship/cli/_initialise.py b/src/virtualship/cli/_initialise.py index 64548b19..409e00e2 100644 --- a/src/virtualship/cli/_initialise.py +++ b/src/virtualship/cli/_initialise.py @@ -67,6 +67,32 @@ def _initialise( click.echo(f"Created '{expedition.name}' at {path}.") +def _parse_mfp_sea_depth(value, waypoint_name: str) -> float | None: + """Sea depth from an MFP export, as a negative elevation in metres.""" + if pd.isna(value): + return None + + match = re.fullmatch(r"~?\s*(-?\d+(?:\.\d+)?)\s*m?", str(value).strip()) + if match is None: + warnings.warn( + f"Could not read the sea depth '{value}' for {waypoint_name}, so it has been left unset. {ERR_SUPPLEMENT}", + stacklevel=2, + ) + return None + + depth = float(match.group(1)) + if depth <= 0: + warnings.warn( + f"{waypoint_name} has a sea depth of '{value}' (at or above sea level, so possibly on land), so it has been left unset.", + stacklevel=2, + ) + return None + + return ( + -depth + ) # MFP reports sea depth as positive, but here we use negative elevations + + def _mfp_to_yaml(file_path: Path, start_date: str, output_path: Path): """Generates an expedition.yaml file from MFP Excel export.""" mfp_data = _validate_mfp_data(file_path) @@ -102,7 +128,19 @@ def _mfp_to_yaml(file_path: Path, start_date: str, output_path: Path): has_latlon = lat is not None and lon is not None waypoints.append(Port(location=loc, time=time_val if has_latlon else None)) else: - waypoints.append(Waypoint(instrument=None, location=loc, time=time_val)) + # waypoint numbers for warning messages, same as the generated expedition.yaml's 'Waypoint #N' order + waypoint_number = sum(isinstance(wp, Waypoint) for wp in waypoints) + 1 + sea_depth = _parse_mfp_sea_depth( + row["Sea Depth"], f"Waypoint {waypoint_number}" + ) + waypoints.append( + Waypoint( + instrument=None, + location=loc, + time=time_val, + sea_depth_meter=sea_depth, + ) + ) previous_timedelta = ( row["Total Time"] if pd.notna(row["Total Time"]) else timedelta(0) diff --git a/src/virtualship/models/expedition.py b/src/virtualship/models/expedition.py index 29700f1e..18ee986e 100644 --- a/src/virtualship/models/expedition.py +++ b/src/virtualship/models/expedition.py @@ -251,6 +251,10 @@ def _annotate(self): waypoint_number += 1 annotated.append(f"{indent}# Waypoint {waypoint_number}\n") + # only written when known (e.g. from an MFP export); omitted means unknown + if wp_data["sea_depth_meter"] is None: + wp_data.pop("sea_depth_meter") + # dump the waypoint on its own so its block can be indented and inserted independently of the others wp_lines = yaml.dump([wp_data], default_flow_style=False).splitlines( keepends=True diff --git a/tests/cli/test_initialise.py b/tests/cli/test_initialise.py index 191d87a4..566700e8 100644 --- a/tests/cli/test_initialise.py +++ b/tests/cli/test_initialise.py @@ -1,7 +1,11 @@ import pandas as pd import pytest -from virtualship.cli._initialise import _load_mfp_export, _mfp_to_yaml +from virtualship.cli._initialise import ( + _load_mfp_export, + _mfp_to_yaml, + _parse_mfp_sea_depth, +) from virtualship.models import Expedition, Port, Waypoint from virtualship.utils import _get_example_expedition @@ -163,6 +167,90 @@ def test_mfp_to_yaml_success(request, fixture_name, tmp_path): assert isinstance(data.schedule.waypoints[1], Waypoint) +def test_mfp_sea_depth_is_positive_and_negated_on_ingest( + valid_excel_mfp_file, tmp_path +): + """Ingest positive MFP sea depths and negate in VirtualShip internal representation.""" + # MFP export gives depths as positive metres below the surface + source_depths = valid_mfp_data()["Sea Depth"] + assert (source_depths > 0).all(), ( + "MFP appears to not be reporting sea depths as positive values, which is a change in behaviour relevant to the VirtualShip workflow (expects positive which are then negated in VirtualShip). Please check the MFP export." + ) + + yaml_output_path = tmp_path / "expedition.yaml" + _mfp_to_yaml(valid_excel_mfp_file, "2023-10-20 01:00:00", yaml_output_path) + + expedition = Expedition.from_yaml(yaml_output_path) + waypoints = [wp for wp in expedition.schedule.waypoints if isinstance(wp, Waypoint)] + + # sign flipped on the way in, magnitude preserved, and the order is kept + station_depths = source_depths[1:-1].tolist() # excluding the two port rows + assert [wp.sea_depth_meter for wp in waypoints] == [-d for d in station_depths] + assert all(wp.sea_depth_meter < 0 for wp in waypoints) + + +def test_mfp_sea_depth_absent_leaves_waypoint_depth_unset(tmp_path): + """A blank sea depth is left as None, and not read as e.g. a depth of zero.""" + df = valid_mfp_data() + df["Sea Depth"] = [None] * len(df) + mfp_file = tmp_path / "no_depths.xlsx" + df.to_excel(mfp_file, index=False) + + yaml_output_path = tmp_path / "expedition.yaml" + _mfp_to_yaml(mfp_file, "2023-10-20 01:00:00", yaml_output_path) + + expedition = Expedition.from_yaml(yaml_output_path) + waypoints = [wp for wp in expedition.schedule.waypoints if isinstance(wp, Waypoint)] + + assert waypoints, "expected the fixture to produce waypoints, but got none" + assert all(wp.sea_depth_meter is None for wp in waypoints) + + +def test_mfp_sea_depth_read_from_export_text_format(tmp_path): + """MFP exports write sea depth as text such as '~2865m', which is read as a negative depth.""" + df = valid_mfp_data() + df["Sea Depth"] = [f"~{d}m" for d in df["Sea Depth"]] + mfp_file = tmp_path / "text_depths.xlsx" + df.to_excel(mfp_file, index=False) + + yaml_output_path = tmp_path / "expedition.yaml" + _mfp_to_yaml(mfp_file, "2023-10-20 01:00:00", yaml_output_path) + + expedition = Expedition.from_yaml(yaml_output_path) + waypoints = [wp for wp in expedition.schedule.waypoints if isinstance(wp, Waypoint)] + assert [wp.sea_depth_meter for wp in waypoints] == [-200.0, -300.0, -400.0] + + +@pytest.mark.parametrize( + "value,expected", + [ + ("~2865m", -2865.0), + ("~ 73 m", -73.0), + ("2865", -2865.0), + (2865, -2865.0), + (152.5, -152.5), + (None, None), + ], +) +def test_parse_mfp_sea_depth(value, expected): + assert _parse_mfp_sea_depth(value, "Waypoint 1") == expected + + +@pytest.mark.parametrize( + "value,match", + [ + ("~-9m", "at or above sea level"), + ("~0m", "at or above sea level"), + ("deep", "Could not read the sea depth"), + ], +) +def test_parse_mfp_sea_depth_unusable_left_unset_with_warning(value, match): + """Unreadable or non-positive (on/above sea level) depths are left unset, with a warning naming the waypoint.""" + with pytest.warns(UserWarning, match=match) as record: + assert _parse_mfp_sea_depth(value, "Waypoint 1") is None + assert "Waypoint 1" in str(record[0].message) + + @pytest.mark.parametrize( "fixture_name,error,match", [ diff --git a/tests/expedition/test_expedition.py b/tests/expedition/test_expedition.py index 81d73aab..7d23e691 100644 --- a/tests/expedition/test_expedition.py +++ b/tests/expedition/test_expedition.py @@ -471,6 +471,47 @@ def test_annotate_no_duplicate_comments(base_expedition, waypoints) -> None: assert len(set(comments)) == len(comments) +def test_sea_depth_only_written_when_known(tmp_path, base_expedition) -> None: + """An unknown sea depth is left out of expedition.yaml entirely (rather than written as null), and a known one is kept.""" + known = Waypoint( + location=Location(0, 0), + time=datetime(2022, 1, 1), + instrument=[InstrumentType.CTD], + sea_depth_meter=-1500.0, + ) + unknown = Waypoint( + location=Location(0, 0.01), + time=datetime(2022, 1, 2), + instrument=[InstrumentType.CTD], + ) + expedition = _with_schedule( + base_expedition, + Schedule( + waypoints=[ + Port(location=None, time=None), + known, + unknown, + Port(location=None, time=None), + ] + ), + ) + + out_path = tmp_path / EXPEDITION + expedition.to_yaml(out_path) + text = out_path.read_text() + + assert text.count("sea_depth_meter") == 1 + assert "sea_depth_meter: -1500.0" in text + # ports' null location/time are still written, so placeholders remain visible + assert "location: null" in text + + reloaded = Expedition.from_yaml(out_path) + assert [wp.sea_depth_meter for wp in reloaded.schedule.waypoints[1:3]] == [ + -1500.0, + None, + ] + + def test_wps_in_use(base_expedition): """Test that _get_wps_in_use() correctly returns waypoints excluding placeholder ports.""" base_time = datetime.strptime("1950-01-01", "%Y-%m-%d")