feat: Add SBE 911+ HEX reader support - #139
Merged
Merged
Conversation
eleanorfrajka
marked this pull request as draft
August 8, 2026 21:53
eleanorfrajka
marked this pull request as ready for review
August 8, 2026 22:04
Contributor
There was a problem hiding this comment.
Pull request overview
Adds Sea-Bird SBE 911plus/917plus .hex support to the existing SBE HEX reader path, expands parameter/mapping metadata for newly produced variables, and introduces multiple real-world fixtures plus integration tests to validate end-to-end decoding and calibration.
Changes:
- Extend
SbeHexReader/sbe_hex_readerto detect and parse SBE911+ HEX + XMLCON configurations and build calibrated datasets. - Add/adjust parameter metadata and mappings (notably
par, plus updatedoxygen/salinitymetadata). - Add new SBE911+ fixtures and integration tests; adjust reader completeness tests to ignore helper dataclasses.
Reviewed changes
Copilot reviewed 30 out of 31 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
seasenselib/readers/sbe_hex_reader.py |
Implements/extends SBE911+ parsing, XMLCON mapping, and dataset construction. |
tests/readers/test_sbe_hex_reader.py |
Adds extensive SBE911+ unit/integration tests and updates existing expectations. |
tests/test_readers_completeness.py |
Excludes helper dataclasses from “reader class” enforcement. |
seasenselib/parameters.py |
Adds par and updates parameter metadata (incl. oxygen, fluorescence, salinity). |
seasenselib/knowledge/pipeline/metadata_enrichment/parameters_metadata.json |
Mirrors metadata updates for enrichment pipeline. |
seasenselib/knowledge/pipeline/mapping/mappings_default.json |
Adds default name mappings for par. |
seasenselib/knowledge/pipeline/mapping/allowed_parameters.json |
Adds par to allowed parameters. |
tests/readers/fixtures/*.hex, tests/readers/fixtures/*.XMLCON, tests/readers/fixtures/*.xmlcon |
Adds real-world SBE911+ fixtures for test coverage. |
.gitignore |
Ignores .coverage file. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+90
to
+96
| FLUORESCENCE: { | ||
| 'long_name': "Fluorescence", | ||
| 'units': 'mg m-3', | ||
| 'coverage_content_type': 'physicalMeasurement', | ||
| 'standard_name': 'volume_fraction_of_oxygen_in_sea_water' | ||
| 'standard_name': 'mass_concentration_of_chlorophyll_in_sea_water', | ||
| 'measurement_type': 'Measured', | ||
| }, |
Comment on lines
+78
to
+84
| "fluorescence": { | ||
| "coverage_content_type": "physicalMeasurement", | ||
| "long_name": "Fluorescence", | ||
| "measurement_type": "Measured", | ||
| "standard_name": "mass_concentration_of_chlorophyll_in_sea_water", | ||
| "units": "mg m-3" | ||
| }, |
Comment on lines
+1564
to
+1586
| # ----- Primary conductivity (freq 1) ----- | ||
| c1_info = xmlcon_map.sensors.get(("frequency", 1)) | ||
| if c1_info is None or c1_info.sensor_type != "conductivity": | ||
| raise ValueError( | ||
| "No primary conductivity sensor at frequency channel 1 in channel map." | ||
| ) | ||
| # convert_conductivity with scalar=1.0 returns mS/cm for 911+ frequency input. | ||
| cond_primary = conv.convert_conductivity( | ||
| conductivity_count=raw["conductivity"].values, | ||
| temperature=temp_primary, | ||
| pressure=pressure, | ||
| coefs=c1_info.coefficients, | ||
| ) | ||
| _median_cond = float(np.median(cond_primary[np.isfinite(cond_primary)])) | ||
| if not (10.0 < _median_cond < 1000.0): | ||
| warnings.warn( | ||
| f"SBE 911+ primary conductivity has median {_median_cond:.4g} mS/cm, " | ||
| "outside the expected range (10–1000). " | ||
| "Check instrument calibration coefficients.", | ||
| UserWarning, | ||
| stacklevel=2, | ||
| ) | ||
| data_vars["cond"] = (params.TIME, cond_primary) |
Comment on lines
+2428
to
2429
| ds["oxygen"].attrs["units"] = "umol l-1" | ||
| ds["oxygen"].attrs["long_name"] = "Dissolved Oxygen" |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
SbeHexReaderto handle SBE 911plus/917plus.hexfiles alongside the existing SBE37 family, using a single-class family-dispatch architecture.detect_sbe_hex_family(), a new* key = valueheader parser, and an index-keyed xmlcon channel-map reader that correctly handles dual T/C and all 8 auxiliary voltage channels.seabirdscientific's frequency converters (convert_temperature_frequency,convert_pressure_digiquartz,convert_conductivity,convert_sbe43_oxygen), fixing ctdproc's hardcoded channel wiring.System UpLoad Time+ scan index ×sample_interval(derived fromNumber of Scans Averaged / 24 Hz), since 911+ files carry no per-scan"date time"column.FluoroWetlabECO_AFL_FL_Sensoroutput isfluorescence, notchlorophyll.MIXSED2_000.hex, 500 scans, +.xmlcon) and adds an end-to-end integration test against it.Variables produced for a full 911+ configuration (dual T/C, 8 aux channels)
temp,cond,press,temp2,cond2,oxygen,oxygen_ml_l,fluorescence,turbidity,altimeter,transmissometer,volt0_raw(user polynomial / UVP6),volt3_raw(pH raw voltage),nmea_latitude,nmea_longitudeSensors declared
NotInUseare skipped with a logged explanation. Sensors present in the xmlcon but without a conversion implementation store raw voltage and log a warning rather than silently disappearing.Breaking changes
SbeHexReader.format_name()changes from"SeaBird SBE37 HEX"to"SeaBird SBE HEX". Any downstream code that hardcodes the old string to identify this reader will need updating.ValueErrorinstead. Files that previously loaded with wrong silent substitutions will now error; fix: supply the correct xmlcon.SBE37SMis no longer the silent fallback for an unrecognised SBE37 device type — it now raisesValueError. The 911+ path never uses this code.Test plan
pytest tests/readers/test_sbe_hex_reader.py -v— all tests passtests/readers/fixtures/for SBE911+Test fixtures added
All
.hexfiles are trimmed to ~500 scans (header preserved verbatim).MIXSED2_000.hexis the primary end-to-end fixture; the others exercise specific xmlcon configurations.m84_3_287_short.hexCoefficients[@equation]wrapper)64PE426_000_1_short.hexmsm_021_1_168_short.hexMSM72_002_2_short.hexM104_154_01_short.hexPS129_014_01_short.hexP503_358_short.hexM166_025_1_short.hexm212_005_short.hexmsm_142_1_056_short.hexMIXSED2_000.hex