diff --git a/echopype/consolidate/api.py b/echopype/consolidate/api.py index 26f4ea6ba..816005c3d 100644 --- a/echopype/consolidate/api.py +++ b/echopype/consolidate/api.py @@ -20,7 +20,7 @@ ek_use_platform_angles, ek_use_platform_vertical_offsets, ) -from .loc_utils import check_loc_time_dim_duplicates, check_loc_vars_validity, sel_nmea +from .loc_utils import check_and_drop_loc_time_dim_duplicates, check_loc_vars_validity, sel_nmea from .split_beam_angle import get_angle_complex_samples, get_angle_power_samples logger = _init_logger(__name__) @@ -277,7 +277,15 @@ def add_location( Returns ------- The input dataset with the location data added - """ + Returns + ------- + The input dataset with location data added + + Notes + ------ + If duplicated time values are found in the latitude/longitude data, only the first entry is kept + for the interpolation operation, and a warning is issued. + """ # Open dataset and echodata object ds = open_source(ds, "dataset", {}) echodata = open_source(echodata, "echodata", {}) @@ -306,6 +314,27 @@ def add_location( # Copy dataset interp_ds = ds.copy() + # Build contextual warning message for duplicate timestamps. + # In the default NMEA case, multiple sentence types may be mixed, + # which can produce duplicate timestamps due to differing resolution. + extra_msg = "" + + if nmea_sentence is None and datagram_type is None and "sentence_type" in echodata["Platform"]: + sentence_types = np.unique(echodata["Platform"]["sentence_type"].values) + sentence_types = [str(s) for s in sentence_types] + + if len(sentence_types) > 1: + extra_msg = ( + f" Multiple NMEA sentence types detected ({', '.join(sentence_types)}), " + "which may have different resolution and produce duplicate timestamps. " + "Consider specifying `nmea_sentence` to select a single GPS message type. " + "Only the first entry with the same timestamp will be used for interpolation." + ) + elif len(sentence_types) == 1: + extra_msg = ( + f" Duplicate timestamps found within NMEA sentence type {sentence_types[0]}." + ) + # Select NMEA subset (if applicable) and interpolate location variables and place # into `interp_ds`. for loc_name, interp_loc_name in [(lat_name, "latitude"), (lon_name, "longitude")]: @@ -316,8 +345,9 @@ def add_location( datagram_type=datagram_type, ) - # Check if there are duplicates in time_dim_name for this NMEA subset - check_loc_time_dim_duplicates(loc_var, time_dim_name) + # Deduplicate time dimension if needed (e.g. multiple NMEA sentences + # at the same timestamp); required for downstream interpolation. + loc_var = check_and_drop_loc_time_dim_duplicates(loc_var, time_dim_name, extra_msg) interp_ds[interp_loc_name] = align_to_ping_time( loc_var, time_dim_name, ds["ping_time"], "linear" diff --git a/echopype/consolidate/loc_utils.py b/echopype/consolidate/loc_utils.py index a55d0fc81..783c2248e 100644 --- a/echopype/consolidate/loc_utils.py +++ b/echopype/consolidate/loc_utils.py @@ -1,3 +1,4 @@ +import warnings from typing import Union import numpy as np @@ -107,13 +108,25 @@ def check_loc_vars_validity( logger.warning(output_message) -def check_loc_time_dim_duplicates(da: xr.DataArray, time_dim_name: str) -> None: - """Check if there are duplicates in time_dim_name""" - if len(np.unique(da[time_dim_name].data)) != len(da[time_dim_name].data): - raise ValueError( - f'Data contains duplicate time values in time_dim_name "{time_dim_name}". ' - "Downstream interpolation on the position variables requires unique time values." +def check_and_drop_loc_time_dim_duplicates( + da: xr.DataArray, + time_dim_name: str, + extra_msg: str = "", +) -> xr.DataArray: + """Check for and drop duplicates in time_dim_name. + """ + time_vals = da[time_dim_name].data + if len(np.unique(time_vals)) != len(time_vals): + n_total = len(time_vals) + n_unique = len(np.unique(time_vals)) + warnings.warn( + f'Dropped {n_total - n_unique} duplicate value(s) in "{time_dim_name}".' + extra_msg, + UserWarning, + stacklevel=2, ) + _, unique_idx = np.unique(time_vals, return_index=True) + da = da.isel({time_dim_name: np.sort(unique_idx)}) + return da def sel_nmea( diff --git a/echopype/tests/consolidate/test_add_location.py b/echopype/tests/consolidate/test_add_location.py index 04c4fed53..1408172f5 100644 --- a/echopype/tests/consolidate/test_add_location.py +++ b/echopype/tests/consolidate/test_add_location.py @@ -234,10 +234,10 @@ def _tests(ds_test, location_type, nmea_sentence=None): ), ], ) -def test_add_location_time_duplicates_value_error( +def test_add_location_time_duplicates_warning( ek80_path, raw_path, sonar_model, datagram_type, parse_idx, time_dim_name, compute_Sv_kwargs, ): - """Tests for duplicate time value error in ``add_location``.""" + """Tests that duplicate time values are handled with a warning, not an error.""" # Open raw and compute the Sv dataset if parse_idx: ed = ep.open_raw(ek80_path / raw_path, include_idx=True, sonar_model=sonar_model) @@ -255,16 +255,19 @@ def test_add_location_time_duplicates_value_error( vals[0] = vals[1] ed["Platform"] = ed["Platform"].assign_coords({time_dim_name: (da.dims, vals)}) - # Check if the expected error is logged - with pytest.raises(ValueError) as exc_info: - # Run add location with duplicated time - ep.consolidate.add_location(ds=ds, echodata=ed, datagram_type=datagram_type) + # Should succeed with a warning instead of raising ValueError + with pytest.warns(UserWarning, match="Dropped 1 duplicate value"): + ds_loc = ep.consolidate.add_location( + ds=ds, echodata=ed, datagram_type=datagram_type + ) + + # Verify location was successfully added + assert "latitude" in ds_loc + assert "longitude" in ds_loc - # Check if the specific error message is in the logs - assert ( - f'Data contains duplicate time values in time_dim_name "{time_dim_name}". ' - "Downstream interpolation on the position variables requires unique time values." - ) == str(exc_info.value) + # Verify latitude/longitude align with ping_time dimension + assert ds_loc["latitude"].sizes["ping_time"] == ds_loc.sizes["ping_time"] + assert ds_loc["longitude"].sizes["ping_time"] == ds_loc.sizes["ping_time"] @pytest.mark.integration