Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions dascore/core/coordmanager.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
from dascore.constants import dascore_styles, select_values_description
from dascore.core.coords import (
BaseCoord,
CoordPartial,
CoordRange,
CoordSummary,
get_coord,
Expand Down Expand Up @@ -1219,6 +1220,30 @@ def get_coord_manager(
return out


def _canonicalization_moved_values(original, out) -> bool:
"""
Return True if canonicalizing `original` to `out` changed any value.

Collapsing an array coord to a CoordRange is meant to be a change of
representation, but the evenness test behind it is tolerant, so a
coordinate carrying small real irregularity (measured positions, timing
jitter) would be replaced by an idealized ramp. Only the exact case is a
canonicalization; the rest is data loss.
"""
# Only collapsing to a range can move values, and only a coord we were
# handed can be kept, so everything else skips the comparison. A CoordRange
# cannot reach here; the caller returns it before this is consulted.
if original is None or not isinstance(out, CoordRange):
return False
# A CoordPartial is a placeholder whose values are all NaN, so it has
# nothing to lose; canonicalizing it is the whole point.
if isinstance(original, CoordPartial):
return False
# Canonicalization re-labels a coordinate, it never resamples one.
assert original.shape == out.shape
return not np.array_equal(original.values, out.values)


def _get_coord_dim_map(coords, dims):
"""Get coord_map, dim_map, and new dims from coord input."""

Expand All @@ -1233,12 +1258,15 @@ def _get_coord(coord):
# CoordRange -- get_coord performs that inference.
if isinstance(coord, CoordRange):
return coord
original = coord if isinstance(coord, BaseCoord) else None
if hasattr(coord, "model_dump"):
coord = coord.model_dump(exclude_defaults=True)
if isinstance(coord, Mapping): # input is a dict
out = get_coord(**coord)
else:
out = get_coord(data=coord)
if _canonicalization_moved_values(original, out):
return original
return out

def _coord_from_simple(name, coord):
Expand Down
3 changes: 1 addition & 2 deletions dascore/io/febus/g1utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -205,8 +205,7 @@ def _coord(values, units=None):
# stored as end_times: starts and ends are each near-regular and snap to
# slightly different steps, so subtracting the two built coords would turn
# the jitter into a linear drift. Built exactly, and ignoring `snap`,
# because that jitter is the signal -- though a span array regular enough
# to look like a range is still normalized to one by the coord manager.
# because that jitter is the signal.
sample_span = get_exact_coord(dc.to_timedelta64(ends - starts))
distance = _coord(resource["distances"][...], units="m")
temperature = _coord(resource["temperatures"][...], units="°C")
Expand Down
36 changes: 35 additions & 1 deletion tests/test_core/test_coordmanager.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
)
from dascore.core.coords import (
BaseCoord,
CoordMonotonicArray,
CoordPartial,
CoordRange,
get_coord,
Expand Down Expand Up @@ -1068,7 +1069,9 @@ class TestPreserveBaseCoord:
Only CoordRange (the canonical evenly-sampled representation) is returned
unchanged. Other coord types (CoordPartial, CoordArray, CoordMonotonicArray)
are still re-inferred so a fully-specified partial, or slicing that yields an
even/empty subset, is canonicalized.
even/empty subset, is canonicalized. That re-inference keeps the coord it was
given whenever collapsing it to a range would move any value, since only the
exact case is a change of representation rather than of data.
"""

def test_update_preserves_range_coord_identity(self, cm_basic):
Expand Down Expand Up @@ -1135,6 +1138,37 @@ def test_even_subset_of_irregular_coord_canonicalizes(self):
assert isinstance(new_coord, CoordRange)
assert new_coord.evenly_sampled

def test_near_even_coord_keeps_its_values(self):
"""Small real irregularity must survive; see #896.

The evenness test is tolerant (rtol 1e-3), so spacing that varies by
less than that used to be replaced by an idealized ramp -- silently
inventing sample positions.
"""
values = np.array([0.0, 1.0, 2.0005, 3.0015, 4.002])
coord = CoordMonotonicArray(values=values)
cm = get_coord_manager({"distance": coord}, dims=("distance",))
out = cm.coord_map["distance"]
assert np.array_equal(out.values, values)
# a range here would mean the spacing was made up
assert not isinstance(out, CoordRange)

def test_near_even_coord_survives_patch(self):
"""The public Patch path must not move the values either."""
values = np.array([0.0, 1.0, 2.0005, 3.0015, 4.002])
coord = CoordMonotonicArray(values=values)
patch = dc.Patch(data=np.zeros(5), coords={"x": coord}, dims=("x",))
assert np.array_equal(patch.get_coord("x").values, values)

def test_exactly_even_array_still_canonicalizes(self):
"""Preserving values must not disable the lossless collapse."""
values = np.arange(5, dtype=float)
coord = CoordMonotonicArray(values=values)
cm = get_coord_manager({"distance": coord}, dims=("distance",))
out = cm.coord_map["distance"]
assert isinstance(out, CoordRange)
assert np.array_equal(out.values, values)


class TestSqueeze:
"""Tests for squeezing degenerate dimensions."""
Expand Down
20 changes: 20 additions & 0 deletions tests/test_io/test_febus/test_febusbsl.py
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,26 @@ def test_sample_span_differenced_before_snapping(self):
# the snapped time coord would have given a constant span
assert len(np.unique(span.values)) > 1

def test_sample_span_keeps_a_monotonic_drift(self):
"""Spans that drift steadily are regular enough to be snapped away.

They survive because the coord manager no longer collapses a coord
when doing so would move its values (#896).
"""
starts = np.arange(4, dtype=np.float64)
ends = starts + np.array([1.0, 1.001, 1.0020005, 1.0030015])
coords = _get_g1_h5_base_coords(
{
"start_times": starts,
"end_times": ends,
"distances": np.arange(2, dtype=np.float64),
"temperatures": np.zeros(4, dtype=np.float64),
},
dims=("time", "distance"),
)
span = coords.coord_map["sample_span"]
assert np.array_equal(span.values, dc.to_timedelta64(ends - starts))

def test_mismatched_time_dataset_lengths_raise(self, bsl_path, tmp_path):
"""A half-written file should fail loudly, not broadcast to garbage."""
new_path = tmp_path / bsl_path.name
Expand Down
Loading