Skip to content

BDMS 122: lexicon updates & other updates#126

Merged
jirhiker merged 4 commits intopre-productionfrom
jab-lexicon-updates
Sep 10, 2025
Merged

BDMS 122: lexicon updates & other updates#126
jirhiker merged 4 commits intopre-productionfrom
jab-lexicon-updates

Conversation

@jacob-a-brown
Copy link
Contributor

@jacob-a-brown jacob-a-brown commented Sep 10, 2025

Why

This PR addresses the following problem / context:

  • The lexicon table should contain all NM_Aquifer lookup table values so that data can be transfered

How

Implementation summary - the following was changed / added / removed:

  • Added values to core/lexicon.json

Notes

Any special considerations, workarounds, or follow-up work to note?

  • Used full names except for organizations since they are commonly referred to with their abbreviations, like USGS
  • The transfer scripts will need to be updated to use code/meaning mappings so the transferred data will be in the lexicon term table
  • Added some verbiage to the README to help with future model revisions

@jacob-a-brown jacob-a-brown changed the title BDMS 122: lexicon updates BDMS 122: lexicon updates & other updates Sep 10, 2025
@codecov-commenter
Copy link

codecov-commenter commented Sep 10, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines Coverage Δ
tests/conftest.py 100.00% <ø> (ø)
tests/test_observation.py 96.85% <ø> (ø)

... and 3 files with indirect coverage changes

{"categories": [{"name": "sample_type", "description": null}], "term": "Standard field sample", "definition": "Standard field sample"},
{"categories": [{"name": "sample_type", "description": null}], "term": "Soil or Rock sample", "definition": "Soil or Rock sample"},
{"categories": [{"name": "sample_type", "description": null}], "term": "Trip blank", "definition": "Trip blank"},
{"categories": [{"name": "sample_type", "description": null}], "term": "Source water blank", "definition": "Source water blank"}
]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I worry about transferring the formation and lithology fields without doing some cleanup.

For instance, is there a distinction between Sand, silt and gravel and Gravel, sand and silt, or would one record suffice? Doesn't the USGS have a controlled vocabulary for lithology?

Formation records seem a mess

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To facilitate the transfer of data we can keep all of the lookup table values as-is, then we can work to clean up those terms. Since a lexicon_term has onupdate="CASCADE" any changes should propagate to the appropriate places. I could also remove formation and lithology from the lexicon, but I'd be concerned that would inhibit data transfers. (If we wait to record lexicon terms for formation and lithology we could just wait to transfer all formation and lithology values until it's cleaned up and mapped to the new terms.)

Copy link
Contributor

@ksmuczynski ksmuczynski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. I agree that there are several lookup tables that could benefit from a clean up. Modifying the lexicon terms later doesn't seem like a big deal, but what about cases where the cleanup is significant and lexicon terms are removed (but the removed terms are referenced in other tables? Should we set some kind of an ON DELETE constraint or flag it in some other way? Is it important to figure out now or later?

@jacob-a-brown
Copy link
Contributor Author

That's a good point, particularly if some are duplicative and we decide to just keep one (e.g. sand, silt, gravel and gravel, sand, silt).

So remove them for now?

@jirhiker
Copy link
Member

Lets remove for now. We need AMP to review and cleanup. This could be something Kimball could help with

@jirhiker jirhiker merged commit 6ce68bb into pre-production Sep 10, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants