chore(ogc): rich layer and field-level metadata - #872
Merged
jirhiker merged 8 commits intoAug 22, 2026
Conversation
Every collection carried a one-line description that mostly restated its title -- "Trend classification for depth to water based on slope in feet per year" tells a consumer nothing about which measurements went in, how the slope was fitted, or when the answer is trustworthy. Keywords were similarly thin, often three tokens echoing the layer id. Each description now says, in plain language, how the layer is derived and what it is for. The derivation text was written against the view SQL, so the derived layers state the things a consumer cannot otherwise see: * depth_to_water_trend_wells classifies at +/- 0.25 ft/yr and reports "not enough data" below 10 readings (or 4 readings under two years), and an increasing trend means the water table is falling. * water_elevation_wells subtracts depth-to-water from surveyed ground elevation, after converting metric readings to feet. * The chemistry pivots normalize inconsistent legacy analyte names onto one canonical set and keep the latest result per analyte, so analytes at one well can carry different sampling dates. * actively_monitored_wells requires both Water Level Network membership and a current "Currently monitored" status. * The three water-level layers state that a reading with no recorded measuring-point height is treated as taken at ground level. That is what the SQL does today; whether it is the right policy is BDMS-980 (A22) and is not decided here. * The geothermal layers explain BHT, temperature-depth profiles, heat flow and drill stem tests for readers who have not met the terms, and note the Fahrenheit/Celsius mixing the source records carry. The internal mount's copy of each layer gets the same text, and the two internal-only layers say why they are not public: avg_tds_wells warns that its mean rests on ~1.9 analyses per well, and latest_depth_to_water_wells points at water_well_summary. test_every_collection_description_explains_the_layer holds the floor: substantive length, no placeholder wording, complete sentences, and lowercase hyphenated keyword tokens. The token check is not cosmetic -- YAML folds a line break into a space, so a hyphenated word wrapped across lines reaches consumers as "measuring- point". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Collection-level prose landed in the previous commit; below it there is still nothing. A client asking what depth_to_water_bgs or hours_since_circulation means gets back a column name and a JSON type. core/ogc-field-descriptions.yml holds a title, a description, and a unit where one applies, for every column of every published collection. It is keyed by backing relation with the ogc_/ogc_internal_ prefix stripped, so the two mounts share one entry per view and the provider can look itself up from self.table with no new plumbing. A _defaults block covers the columns shared across views -- the 11 thing-type views have one identical 18-column signature between them. Rejected: COMMENT ON COLUMN on the ogc_* views. It would put the prose next to the data, but every wording fix would need an Alembic revision and a matview rebuild, and pygeoapi's reflection does not read comments, so a catalog query would be needed regardless. describe_fields() returns fresh dicts rather than references into the cached YAML. That is load-bearing: pygeoapi's get_collection_schema assigns the provider's own field dict into its response and then mutates it in place, so shared references would let one request's x-ogc-role assignment leak into every later response. A missing entry is a generated title and a logged warning, never an error. Drift is caught by tests, not at runtime. The 190 chemistry analyte columns are generated by cli/generate_chemistry_field_descriptions.py and reviewed by hand. It reads the analyte lists out of the view migration rather than the parameter lexicon, which holds only two field parameters. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Routes all 25 feature collections through a PostgreSQLProvider subclass that annotates the reflected columns from ogc-field-descriptions.yml. pygeoapi's get_collection_schema copies each provider field entry into its response wholesale, so title, description and x-ogc-unit reach the client with no patching. The subclass is four lines longer than it looks like it should be. BaseProvider.fields -- what both /schema and /queryables actually read -- returns self._fields directly and never calls get_fields(), and GenericSQLProvider.__init__ populates _fields with the raw reflection at construction. An override that only returned an annotated copy would be silently discarded, so this one writes back into _fields, with a flag because the SQL implementation short-circuits on a populated cache. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
/schema needed no patch because pygeoapi copies provider field entries wholesale. get_collection_queryables does the opposite: it builds a fresh dict per property and hardcodes 'title': k, the raw column name, so the documentation the provider attaches never survives. Rather than fork the 130-line handler, this wraps it and merges the provider's title, description and unit into the JSON it returned. HTML renders, error statuses and unparseable bodies pass straight through. The cost is a JSON round trip and one extra table reflection on an endpoint that is queried rarely; the benefit is that property filtering, domains, enums and roles stay pygeoapi's code rather than ours. The patch is applied once in _load_pygeoapi_app before the module is executed. It is deliberately not config-dependent: pygeoapi.api.itemtypes is a single module object shared by both mounts, and starlette_app resolves the handler off it per request, so one patch covers both. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The EDR provider already returned a title and unit per parameter; it now routes them through the same YAML the feature collections use, and the CoverageJSON parameters block carries the documentation where a client looks for it -- observedProperty.label for the display name, description for the explanation. Both were the raw parameter name before. EDR fields are keyed by parameter name rather than column name because that is what the provider reports. ogc_waterlevels stamps one literal, 'groundwater level', which is documented here. ogc_water_chemistry carries the analyte text exactly as the laboratory recorded it -- an open-ended set that is only knowable from the data -- so most chemistry parameters take a generated title, which is the designed fallback rather than a gap. tests/test_ogc_field_descriptions.py covers both endpoints on both mounts, the internal-only collection, JSON and HTML rendering, and the fallback path. Two guards earn their keep: * the drift guard fails when a matview column has no YAML entry, so a column rename breaks CI instead of quietly degrading the API. EDR collections are exempt -- their fields are data, not columns. * the upgrade guard asserts pygeoapi still assigns the provider's field entry into the schema response. A bump that rebuilds the dict instead, the way get_collection_queryables already does, would silently drop every description; this turns that into a red test. geometry is excluded from the "every property is documented" assertions: pygeoapi injects it itself, after the provider's fields, carrying only a format and a role. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Records where the YAML lives, how to add a field, and why the copy is keyed by backing relation rather than collection id. The load-bearing section is the pygeoapi one. Five behaviours of 0.24.0 hold this feature up, none of them part of any public contract -- the schema pass-through, the in-place mutation of the provider's field dicts, the queryables rebuild, BaseProvider.fields returning _fields without calling get_fields(), and starlette_app resolving handlers off a shared module. Each is written down with its file and line so an upgrade has something to check against. Also records the COMMENT ON COLUMN rejection, so the next person does not relitigate it from scratch. CLAUDE.md gets a pointer in the existing style. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Cleanup pass on the field-description change. WaterEDRProvider caches its fields and handed the cached dicts straight back, so pygeoapi's in-place mutation of what a provider returns -- it pops format and assigns x-ogc-role while building /schema -- accumulated on the cache and reached the next response. The feature provider was already safe because describe_fields copies; this closes the same hole on the EDR side. The EDR schema test is now data-independent. Parameter fields are read out of the data rather than reflected from columns, so it passed alone and failed in the full suite, where earlier tests had removed the water-level rows it was reading. It skips with a reason when there is nothing to assert against; the CoverageJSON test covers the same lookup without needing any rows. Also confirmed by hand: OpenAPI generation still succeeds against an unreachable database, so the app starts before the views exist. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
pygeoapi's HTML schema table renders a Values column from each property's `enum`, and it was empty for every collection: the SQL provider reports only type and format, and implements no get_domains(), so ?profile=actual-domain has nothing to offer either. Nothing anywhere filled the key in. Field entries can now set it two ways, and the distinction is the point: * `enum:` literally, for values the view's own SQL decides -- trend_category's four CASE outcomes, and the 'C' the geothermal views stamp after converting temperatures. * `enum-lexicon:` naming a category in core/lexicon.json, expanded on the way out, for anything the lexicon governs -- thing_type, release_status, well_construction_method, well_pump_type, and elevation_method. The vocabulary is never copied, so it cannot drift from the file that seeds it, and a category with no terms fails validation at load rather than publishing an empty column. Not populated from SELECT DISTINCT. `enum` is a JSON Schema constraint, not a sample: a value absent from today's data is not thereby invalid. The queryables patch carries `enum` too, but yields to one pygeoapi produced itself, so ?profile=actual-domain still reports the live domain wherever a provider supports it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
jirhiker
changed the title
Contributor
Coverage✅ 79.19% 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.
Stacked on #871 — base is
chore/bdms-977-979-hide-ogc-layers, so this diff excludes the layer-hiding work. GitHub retargets it tostagingonce #871 merges.Two layers of metadata, in one branch because the second is meaningless without the first.
1. Collection-level prose
Every collection carried a one-line description that mostly restated its title. "Trend classification for depth to water based on slope in feet per year" tells a consumer nothing about which measurements went in, how the slope was fitted, or when the answer can be trusted. Keywords were often three tokens echoing the layer id.
All 27 collection entries rewritten — 14 in
core/pygeoapi-config.yml, 11 incore/pygeoapi-config-internal.yml, 11 thing-type + 2 EDR incore/pygeoapi.py— each saying how the layer is derived and what it is for, written against the view SQL rather than paraphrased from the old copy.Things a consumer could not previously discover: that an increasing depth-to-water trend means the water table is falling; that the chemistry pivots keep the latest result per analyte, so one well's analytes can carry different dates; that
actively_monitored_wellsneeds both network membership and a current status; that the geothermal source records mix Fahrenheit and Celsius.2. Field-level descriptions
Below the collection there was nothing at all — a client asking what
depth_to_water_bgsorhours_since_circulationmeans got back a column name and a JSON type.core/ogc-field-descriptions.ymlnow holds a title, description, and unit for every column of every published collection, served through the standard endpoints:GET /collections/{id}/schematitle,description,x-ogc-unit,x-ogc-unitLang,enumGET /collections/{id}/queryablesparametersobservedProperty.labelanddescriptionKeyed by backing relation with the
ogc_/ogc_internal_prefix stripped, so both mounts share one entry per view. A_defaultsblock covers the shared columns — the 11 thing-type views have one identical 18-column signature between them. The 190 chemistry analyte columns are generated bycli/generate_chemistry_field_descriptions.pyand hand-reviewed.Enumerated values
pygeoapi's HTML schema view renders a Values column from each property's
enum, and it was empty for every collection — the SQL provider reports only type and format and implements noget_domains(). Entries can now set it either literally (for values the view's own SQL decides, liketrend_category's fourCASEoutcomes) or viaenum-lexicon:, naming a category incore/lexicon.jsonthat is expanded at read time so the vocabulary is never copied.Deliberately not populated from
SELECT DISTINCT:enumis a JSON Schema constraint, not a sample, and a value absent from today's data is not thereby invalid.What this leans on inside pygeoapi
The feature depends on five unpinned behaviours of the pinned 0.24.0 (which is the current PyPI latest — there is no upgrade available). All are written down with file and line in
docs/ogc-field-descriptions.md. Two bit during implementation:BaseProvider.fieldsreturnsself._fieldsand never callsget_fields(), whileGenericSQLProvider.__init__populates it at construction. Aget_fields()override that only returned an annotated copy would have been silently discarded and shipped nothing. The provider writes back into_fields.get_collection_schemamutates the provider's field dicts in place while building its response. Both providers therefore hand out fresh dicts; the EDR one did not, and its cache was accumulatingx-ogc-rolekeys across requests — fixed here./schemaneeds no patch because pygeoapi copies provider field entries wholesale./queryablesrebuilds each property and hardcodes'title': k, socore/pygeoapi_patches.pywraps it and merges the documentation back in, yielding to anyenumpygeoapi produced itself so?profile=actual-domainstill wins where a provider supports it.Tests
tests/test_ogc_field_metadata.py(unit, no database) andtests/test_ogc_field_descriptions.py(both endpoints, both mounts, JSON and HTML, the internal-only collection, EDR, fallback). Two guards earn their keep:get_collection_queryablesalready does, would silently drop every description.geometryis excluded from the "every property is documented" assertions: pygeoapi injects it after the provider's fields with only a format and a role.Notes for review
core/pygeoapi-config-internal.ymlgets the same collection prose, and the two internal-only layers say why they are not public:avg_tds_wellswarns its mean rests on ~1.9 analyses per well,latest_depth_to_water_wellspoints atwater_well_summary.Verification
uv run pytest --ignore=tests/transfers→ 1094 passed, 81 skipped, 6 xpasseduv run behave tests/features/ogc-cleanup-sprint1.feature --tags="@backend and @production and not @skip"→ 20 passed, 0 failedpre-commit(black, flake8) clean🤖 Generated with Claude Code