feat(ingestion): add the Van Essen domain rules and adapter - #846
Merged
Conversation
Turns diver readings into Ocotillo values: timestamps to timezone-aware UTC,
centimetres to feet, and deterministic external keys. Pure functions, no
database and no network, per ADR4 -- asserted by a test that checks the import
graph rather than by reading the file.
Narrower than the plan described. The draft had this converting drillingDepth
and building a point from lat/lng, but the live MonitoringPoint payload is
{id, name}: no depth, no coordinates. Those functions would have had no input,
so the plan is corrected rather than the functions written.
The adapter refuses a row whose reference is not ground surface, and one whose
unit is not centimetres. Both would otherwise produce plausible numbers instead
of an error -- the datum is chosen at request time and is not recoverable from
the row, and an unconverted centimetre value reads as a believable depth while
being wrong by a factor of 30.48.
External keys use the vendor's numeric id, not the name. SO-0125 is a Bureau
point id and can be corrected; the numeric id is what the vendor's URLs use and
what a re-run has to resolve to the same record.
Negative depths are kept: water stands above ground in these riparian wells at
high flow, and clamping would erase real data.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
Coverage✅ 79.63% total — gate is 75%. Coverage for the Python files changed in this PR
|
Contributor
|
Your pull request is automatically being deployed to Dagster Cloud.
|
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.
Task 3.1.
domain/van_essen.pyplussources/san_acacia/adapter.py— 28 newtests, no database and no network.
Narrower than the plan said
The draft had this converting
drillingDepthfrom centimetres and building aWGS84 point from
lat/lng. The liveMonitoringPointpayload is{id, name}— no depth, no coordinates — so those functions would have had noinput. Geometry and construction come from the Ocotillo records a point
reconciles against, which is consistent with ingestion never creating wells.
I corrected the plan rather than writing functions with no callers.
What it does
documents UTC and does not always mark it. Reading it as local would shift
every observation by the machine's offset — and differently on a laptop than
in a container.
domain/units.convert_cm_to_ft.SO-0125is a Bureau point id and can be corrected; the numeric id is whatthe vendor's URLs use and what a re-run must resolve to the same record.
ValueError, matching the per-row contract the CSV importersalready expect.
ADR4 layering is asserted by a test that inspects the import graph, rather than
by reading the file: no
fastapi,sqlalchemy,pydantic,httpx,db,api,schemas, orservices.What the adapter refuses
Both cases would otherwise produce plausible numbers rather than an error, which
is the failure mode this source is most prone to:
reference != 3unit != "cm"Per-record failures are collected, not raised — one unparseable reading costs
that reading, not the series.
Judgement calls worth review
Negative depths are kept. Depth below ground goes negative when water stands
above ground, which happens in these riparian wells at high flow. Clamping would
erase real data.
Nonelevels pass through as gaps, but the adapter then refuses the row —there is nothing to store. That is a failure entry, not silence, so gaps show up
in the metadata count.
The module docstring lists every value the mapping invents rather than
reads — datum, unit, timezone — since inventing is where a mapping goes quietly
wrong.
Note
Running
tests/locally needsMODE=development; a.envwithAUTHENTIK_DISABLE_AUTHENTICATION=1andMODE=stagingaborts collection. Thatis pre-existing and unrelated. 77 tests pass across both directories.
🤖 Generated with Claude Code