feat(chemistry): serve legacy water chemistry over REST - #870
Merged
Conversation
Adds GET /chemistry/results, one row per analyte, and registers the chemistry router -- it existed with every route commented out, so the API served no chemistry at all. The water chemistry is in the legacy NMA tables, not in the refactored `observation` table, which holds none of it. Rather than read those four tables directly, this serves `ogc_water_chemistry`, the view d9e0f1a2b3c4 already built by unioning them for the OGC EDR mount. Same rows, one definition of what a chemistry result is. Only the public view is served, so an unreleased thing or a sample flagged PublicRelease = false is not reachable here regardless of who asks. Analytes are stored as legacy symbols (`As`, `SO4`, `pHf`) and are translated to the lexicon's parameter names on the way out, because that is what callers key on to match a result to a drinking water standard. Doing it per caller means each one gets to be wrong separately. Ambiguous symbols are deliberately left untranslated so nothing can act on a guess. `NO3` maps to the as-NO3 name rather than the as-N one: the nitrate MCL is 10 mg/L as N, about 45 mg/L as NO3, so collapsing the two would flag wells that are nowhere near the limit. The legacy data records `NO3(N)` separately and that is what carries the as-N name. `start_time` is inclusive and `end_time` exclusive so a calendar year is expressible without picking up New Year's Day of the next one, and paging breaks ties on id so analytes sharing a timestamp cannot be served twice or skipped. Refs BDMS-1189 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Adds `result_kind` (major, minor, radionuclide, field) to the chemistry results response. Whether a result was read at the wellhead or by a laboratory is a distinction an owner-facing report has to draw, and the legacy tables are the only record of it -- the refactored `parameter_type` it used to come from is not populated for this data. The view keeps that provenance only in its text id prefix, so the prefix is translated here into a field a client can read instead of every client learning to parse an id. Refs BDMS-1189 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
GET /thing/water-well returned 500 for any page containing a thing with no current location: `current_location` was a required field, so the GeoJSON validator was handed None, reached for `__table__` on it, and raised AttributeError. One unlocated well made every well on its page unreadable. The dev database has 49 of them, so the listing failed at any page size that reached one. A thing is associated with a location over an effective period, and that period can be closed or never opened, so having no current location is a state the schema has to be able to say. The field is now optional and the validator hands None back for the annotation to resolve. Found while pointing the chemistry report at this endpoint; the bug is older than that and affects every consumer of the wells listing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Working on the legacy chemistry REST routes means having legacy chemistry to
read, and until now the only way to get it into ocotilloapi_test was a SQL Server
connection or hand-written rows. Tests build their own single records inline,
which is right for unit tests but leaves nothing to exercise the normalized
chemistry views, the LIMS ingestion path, or a list endpoint against realistic
analyte, unit and detection-limit distributions.
This copies a bounded subset out of a local clone of another database (default
ocotillo_prod) into ocotilloapi_test, walking the dependency closure:
thing -> location + location_thing_association
-> NMA_Chemistry_SampleInfo
-> NMA_MajorChemistry
-> NMA_MinorTraceChemistry
The association rows matter as much as the locations. thing.nma_pk_location is a
legacy audit column, not the live link: the model reaches a location through
location_thing_association (Thing.location_associations), so seeding a thing
without association rows produces exactly the location-less well that bff7faa
had to stop 500ing on.
Primary keys are deliberately not preserved. The target already holds unrelated
rows at low ids, so copying source ids verbatim would silently reparent sample
infos onto pre-existing test things wherever the ranges overlap. Rows are
inserted without an id and children are repointed at the new parent id.
Re-runs reconcile on the legacy natural keys rather than on ids: a sample info
whose nma_SamplePtID is already present is skipped, and location/thing are reused
via nma_pk_location/nma_pk_welldata. So a second run picks up the next unseeded
batch instead of duplicating the last one.
Candidates must have both a major and a minor/trace row, so every seeded sample
exercises both tables. thing.thing_type is a NOT NULL lexicon FK, so a thing
whose type the target lexicon lacks disqualifies its sample infos rather than
being patched; nullable lexicon-backed columns are nulled with a count reported.
Column sets are intersected per run, so schema drift between the two databases
degrades to a printed skip list instead of a crash. thing.search_vector is
skipped as trigger-maintained, and location.point round-trips as EWKT because
pg8000 carries no geometry codec.
The target name must contain 'test' unless --force is passed.
The seed is transient by design of the test suite: the session-scoped autouse
fixture in tests/conftest.py drops and re-migrates the schema, so any pytest run
wipes it and the script has to be re-run afterwards. That also means it cannot
perturb the suite -- pytest always starts from a clean schema. Documented in the
module docstring.
Verified by seeding 65 sample infos across two runs (764 major and 1223
minor/trace rows, 22 and 79 distinct analytes, 437 censored values): the second
run reported 60 already-seeded candidates skipped, natural keys stayed unique
(65/65 sample infos, 764/764 major GlobalIDs), no chemistry row was orphaned, all
60 chemistry things resolved a location through the association table, and every
copied location kept its geometry.
Depends on a local database clone, so it is a dev-box seeder and does not run in
CI.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
Coverage✅ 79.12% total — gate is 75%. Coverage for the Python files changed in this PR
|
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
1 participant
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.
What
Four commits, three shipped earlier in the branch plus one from this session:
97d7f9fbserve legacy water chemistry over REST —services/legacy_chemistry.pyreadsNMA_MajorChemistry/NMA_MinorTraceChemistryand exposes results over the API.67b522careport which legacy table a result came from — callers can tell a major-ion result from a minor/trace one.bff7faa9a well with no location no longer 500s the listing —schemas/thing.py, covered bytests/test_thing_without_location.py.8e55d1d4seed the test DB with real NMA legacy chemistry —scripts/seed_nma_chemistry.py(new, this session).Why the seeder
Working on the legacy chemistry routes means having legacy chemistry to read, and the only ways to get it into
ocotilloapi_testwere a SQL Server connection or hand-written rows. Tests build single records inline — right for unit tests, but nothing there exercises the normalized chemistry views, the LIMS path, or a list endpoint against realistic analyte / unit / detection-limit distributions.The script copies a bounded subset out of a local clone of another database (default
ocotillo_prod) intoocotilloapi_test, walking the dependency closure:Reviewer notes
thing.nma_pk_locationis a legacy audit column, not the live link — the model reaches a location throughlocation_thing_association(Thing.location_associations). Seeding a thing without association rows produces exactly the location-less well thatbff7faa9in this same branch had to stop 500ing on. First cut of the script got this wrong; caught by inspecting the seeded rows, fixed before commit.nma_SamplePtIDfor sample infos,nma_pk_location/nma_pk_welldatafor location and thing. A second run picks up the next unseeded batch rather than duplicating the last.thing.thing_typeis a NOT NULL lexicon FK, so a thing whose type the target lexicon lacks disqualifies its sample infos instead of being patched. Nullable lexicon-backed columns are nulled with a count printed.b6c7d8e9f0a1against the test DB'sc3d4e5f6a7b8with no drift in the five tables involved.thing.search_vectoris skipped (trigger-maintained);location.pointround-trips as EWKT because pg8000 has no geometry codec.test, or--forceis passed.The seed is transient
tests/conftest.pyhas a session-scoped autouse fixture that drops and re-migrates the schema, so anypytestrun wipes the seeded rows and the script has to be re-run. The flip side is that it cannot perturb the suite — pytest always starts from a clean schema. Noted in the module docstring.Testing
Seeded 65 sample infos across two runs — 764 major and 1223 minor/trace rows, 22 and 79 distinct analytes, 437 censored (
Symbolnon-null) values:GlobalIDstest_major_chemistry_legacy,test_nma_chemistry_lineage,test_chemistry_lims(57 passed) andtest_ogc(22 passed, 2 skipped) are green. Note those runs reset the schema first, so they show the suite is unaffected — they do not exercise the seeded rows.The seeder depends on a local database clone, so it is a dev-box tool and does not run in CI.
🤖 Generated with Claude Code