BDMS 233: transfer updates#261
Conversation
…s-227-231-additional-well-info-models # Conflicts: # .pre-commit-config.yaml # core/lexicon.json # tests/features/environment.py
…abulary fields - Implemented `AquiferSystem` model for master reference of aquifer systems and hydrogeologic units - Added table index for aquifer system name - Included categories and terms for aquifer type and significance level
jacob-a-brown
requested review from
TylerAdamMartinez,
chasetmartin and
ksmuczynski
jirhiker
left a comment
jirhiker
left a comment
There was a problem hiding this comment.
Looks good. I will need to refactor slightly to match the latest transfer branch
| wdf = read_csv("WellData", dtype={"OSEWelltagID": str}) | ||
| wdf = replace_nans(wdf) | ||
|
|
||
| transferred_wells = ( |
There was a problem hiding this comment.
all the other transfers use filter_to_valid_point_ids for this functionality
There was a problem hiding this comment.
The permissions require the id of the thing record to set the target_id field, which is why the wells are queried in permissions_transfer.py (after the wells have been transferred and assigned ids by the database). Using filter_to_valid_point_ids would get me the correct thing.name for all of the wells, but wouldn't fetch the ids.
There was a problem hiding this comment.
When I was figuring out this transfer it made me sense to me to do it this way, but I can move this to well_transfer.py after line 365 - the second after the comment "add things that need well id"
There was a problem hiding this comment.
Upon reflection I think it makes more sense to go there. It'll be better organized and easier to follow
There was a problem hiding this comment.
I forgot another reason why permission_transfer.py is its own file... The permissions require an associated contact, so they need to be transferred after both the things and contacts are transferred (which is why they can't go into well_transfer.py (which negates the above two comments)
| ) | ||
| for note_type, note_content in location_notes.items(): | ||
| if not isna(note_content): | ||
| location.add_note(note_content, note_type) |
There was a problem hiding this comment.
Doesn't add_note just create a note object? you still need to add it to the session
There was a problem hiding this comment.
I just looked at it and it does - I'll fix to add it to the session
|
|
||
| # WaterLevels.SiteNotes --> notes where note_type = "measuring_notes" | ||
| if not pd.isna(row.SiteNotes): | ||
| note = thing.add_note( |
There was a problem hiding this comment.
how is this note going to be associated with this water level measurement?
There was a problem hiding this comment.
It won't be associated with water level measurements in Ocotillo, but with wells. The step from well-notes.feature indicates the response should include measuring notes (notes about measuring/visiting the well, on Access form). This is really the only field in the database that explicitly records notes about measures/visits, so it is now being associated with a well and not an observation.
The documentation is: "Optional notes concerning the nature of the well site or measuring event."
There was a problem hiding this comment.
I have messages out to Ethan and Sianin to clarify "measuring notes" since this field they mention that they want to see and enter data into is not found specifically in NM Aquifer and needs clarification. Looking at some of the values in WaterLevels.SiteNotes, they are definitely observation level, containing info like battery levels, which seems most relevant when associated with a specific time/date of observation.
I assume this response field from the old AMP API is the WaterLevels.SiteNotes?
There was a problem hiding this comment.
That's correct - that response comes from WaterLevels.SiteNotes
There was a problem hiding this comment.
as @chasetmartin mentioned WaterLevel.SiteNotes is related to observation level notes where time/date info is necessary.
I think the feature is saying return all the observation notes for this well, i.e. all the WaterLevel.SiteNotes. So WaterLevel.SiteNotes needs to be associated with a measurement. The API then collects all these notes and returns them with the other well metadata
There was a problem hiding this comment.
Got it. I'll then update this transfer, as well as update the feature file/test to reflect this
| types: [python] # Specify relevant file types for your tests | ||
| pass_filenames: false | ||
| always_run: true | ||
| # - repo: local |
There was a problem hiding this comment.
You should not be committing these temporary, dev changes to .pre-commit-config.yaml
instead of a specific permission type, return a list of permission records for all permission types associated with the well.
…s-to-pass-tests well additional information testing/implementation updates
…pdates Bdms 227 transfer updates
regex is a more robust way to check for the presence of pump types in construction notes, handling variations in spacing and punctuation.
check to make sure that permissions are set before transferring them from one entity to another to avoid potential errors.
use the polymorphic table rather than a standalone field
This is the name used for serialization so there should be correspondence for ease of use and maintenance
|
This PR is out of date. See PR #272 |
4 participants
Why
This PR addresses the following problem / context:
How
Implementation summary - the following was changed / added / removed:
Other,Water,Construction,MeasuringOther,Coordinatething.well_construction_notes-->note_type = "Construction"location.nma_notes_location-- >note_type = "Other"location.nma_coordinate_notes-->note_type = "Coordinate"well-notes.pyfeature test for the refactoring ofwell_construction_notesto the polymorphic notes tableNotes
Any special considerations, workarounds, or follow-up work to note?
Measuringnotes for athingcome fromWaterLevels.SiteNotesin NM_Aquifer. The documentation for that field states "Optional notes concerning the nature of the well site or measuring event."