fix: Conductivity unit handling: verify in readers, normalise in pipeline

[eleanorfrajka]

Summary

SeaBird SBE37 ASCII files output conductivity in S/m (~3.5), not mS/cm (~35). The values differ by a factor of 10 and are both plausible-looking, so a unit mismatch propagates silently into derived quantities.

Design decision — readers stay clean, pipeline converts: Each reader reports the unit exactly as the instrument file specifies it. SBE37 ASCII outputs S/m; CNV files carry whatever pycnv reads from the channel name. Readers also call infer_conductivity_unit() to cross-check the declared unit against the actual data magnitude and warn on disagreement, but they do not modify values. The alternative — converting in every reader — would scatter the same mS/cm conversion across a growing number of reader classes. Centralising it in a single pipeline handler makes the rule easy to find, test, and extend to new readers without touching existing ones.

Reader changes (verify, don't convert): Readers now call infer_conductivity_unit() and report the accurate unit from the file. For SBE37 ASCII that is "S/m"; for CNV files it is whatever pycnv parses from the channel name ("mS/cm" or "S/m").

New pipeline handler (ConductivityNormalizer): A new conductivity_normalize handler in the unit_handling stage converts all conductivity variables to mS cm-1 before the derivation stage runs. It uses infer_conductivity_unit() + to_mS_cm() from the new readers/utils/conductivity_units.py helper, stores a conductivity_normalised_from provenance attribute, and is a no-op if the variable is already in mS cm-1. Enabled by default in the default and full pipeline profiles. All three unit-change paths — string relabelling (UnitNormalizer), value conversion (ConductivityNormalizer), and pint-based conversion (UnitConverter) — all write into context.metadata['unit_conversions'].

Derivation guard: The derivation stage now checks all conductivity variables at entry and emits a UserWarning if any carry "S/m" or "S m-1" units, naming the handler to enable.

Unit normalizer improvements: Added relabelling rules "S/m" → "S m-1" and "mS/cm" → "mS cm-1" to unit_normalizations.json. Added an elif branch to UnitNormalizer.normalize() so it also compares a declared unit against the expected unit when both are present (previously it only checked when units were absent).

Knowledge file updates: Conductivity canonical unit changed from "S m-1" to "mS cm-1" in parameters.py, parameters_metadata.json, and expected_units.json. Temperature and potential temperature expected units corrected from "K" to "degree_C" in expected_units.json. Removed erroneous "Sverdrup/m": "Sv/m" entry from unit_normalizations.json (Sverdrup is volume transport, not conductance). Temperature units in parameters.py and parameters_metadata.json updated from "ITS-90, deg C" / "degC" to "degree_C" (CF canonical); a comment records the ITS-90 assumption and links to the CCHDO convention.

Coverage config: Added [tool.coverage.run] and [tool.coverage.report] to pyproject.toml so that pytest --cov=seasenselib reports only seasenselib/ lines and omits tests/.

New files

• seasenselib/readers/utils/conductivity_units.py — infer_conductivity_unit(values, declared) and to_mS_cm(values, from_unit) helpers
• seasenselib/pipeline/unit_handling/handlers/conductivity_normalizer.py — ConductivityNormalizer handler
• tests/readers/utils/test_conductivity_units.py — 13 unit tests for the helpers
• tests/readers/fixtures/4049_caldip_short.asc — SBE37-SM ASCII fixture (SN 4049)
• tests/readers/fixtures/7508_caldip_short.hex — SBE37-SM hex fixture (SN 7508)
• tests/readers/fixtures/7508_caldip.xmlcon — companion calibration for 7508 hex

[Copilot]

Pull request overview

Fixes silent conductivity unit mismatches (notably SBE37 ASCII reporting S/m vs mS/cm) by having readers verify units against data magnitude while centralizing actual conductivity value conversion in the pipeline’s unit_handling stage, ensuring derived quantities don’t silently end up off by ×10.

Changes:

• Added conductivity unit inference/conversion helpers and corresponding unit tests + real SBE fixtures.
• Introduced ConductivityNormalizer pipeline handler to normalize conductivity values to canonical mS cm-1 before derivation, plus a derivation-stage guard warning.
• Updated unit normalization/knowledge metadata (conductivity canonical unit, temperature expected units) and pipeline profiles; added coverage configuration.

Reviewed changes

Copilot reviewed 24 out of 24 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
tests/readers/utils/test_conductivity_units.py	Unit tests for conductivity unit inference/conversion helpers
tests/readers/test_sbe_hex_reader.py	Integration tests asserting HEX reader/pipeline conductivity units and value ranges
tests/readers/test_sbe_ascii_reader.py	ASCII reader integration tests for conductivity unit reporting and pipeline normalization
tests/readers/fixtures/7508_caldip.xmlcon	Added SBE37 XMLCON fixture for HEX integration testing
tests/readers/fixtures/7508_caldip_short.hex	Added SBE37 HEX fixture for integration testing
tests/readers/fixtures/4049_caldip_short.asc	Added SBE37 ASCII fixture for integration testing
tests/pipelines/unit_handling/test_unit_handling_handlers.py	Expanded handler tests; added ConductivityNormalizer coverage
tests/pipelines/test_pipeline_profiles.py	Ensures default profile includes conductivity normalization handler
seasenselib/readers/utils/conductivity_units.py	New helper for magnitude-based unit inference and conversion to mS cm-1
seasenselib/readers/utils/init.py	Exposes new conductivity helper utilities
seasenselib/readers/sbe_cnv_reader.py	Verifies declared conductivity units vs magnitude (warn-only)
seasenselib/readers/sbe_ascii_reader.py	Sets SBE37 ASCII conductivity units to S/m and verifies magnitude
seasenselib/pipeline/unit_handling/stage.py	Wires in ConductivityNormalizer and records relabels/conversions metadata
seasenselib/pipeline/unit_handling/handlers/unit_normalizer.py	Renames conversion records to relabels + adds declared-vs-expected check
seasenselib/pipeline/unit_handling/handlers/conductivity_normalizer.py	New handler converting conductivity values to mS cm-1
seasenselib/pipeline/unit_handling/handlers/init.py	Exports ConductivityNormalizer
seasenselib/pipeline/derivation/stage.py	Adds guard warning if conductivity arrives in S/m units
seasenselib/parameters.py	Updates canonical units for conductivity + temperature-related parameters
seasenselib/knowledge/pipeline/unit_handling/unit_normalizations.json	Adds conductivity relabel rules; removes incorrect Sverdrup normalization
seasenselib/knowledge/pipeline/unit_handling/expected_units.json	Updates expected conductivity + temperature expected units
seasenselib/knowledge/pipeline/metadata_enrichment/parameters_metadata.json	Updates canonical units for conductivity/temperature-related params
seasenselib/config/pipeline/full.json	Enables conductivity_normalize in full pipeline profile
seasenselib/config/pipeline/default.json	Enables conductivity_normalize in default pipeline profile
pyproject.toml	Adds coverage configuration for pytest --cov

Suppressed comments (1)

seasenselib/readers/utils/conductivity_units.py:93

• The out-of-range error message hardcodes uS/cm as 500–1000000, but the intended uS/cm inference band (per the module docstring and the ranges table) is ~10000+. Keeping this consistent avoids misleading diagnostics when inference fails.

        raise ValueError(
            f"Cannot infer conductivity unit: median {median:.4g} fits no known "
            f"range (S/m 0.5–10, mS/cm 10–1000, uS/cm 500–1000000). "
            f"Check the data or supply units explicitly."

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[ysorge]

@copilot resolve the merge conflicts in this pull request

[Copilot]

@copilot resolve the merge conflicts in this pull request

Resolved in 7b5166c.