Conversation
… CREATE response
…he GET test for location
db/location.py
Outdated
| elevation_accuracy: Mapped[float] = mapped_column(nullable=True) | ||
| elevation_method: Mapped[str] = lexicon_term(nullable=True) | ||
| coordinate_accuracy: Mapped[float] = mapped_column(nullable=True) | ||
| coordinate_accuracy_unit: Mapped[str] = lexicon_term(nullable=True) |
There was a problem hiding this comment.
Will all elevation_accuracy values be in meters? If so, should elevation_accuracy be renamed elevation_accuracy_m? Or, should we add a field called elevation_accuracy_unit?
Same for coordinate_accuracy: will it always be in degrees since we are storing coordinate values in WGS84.
There was a problem hiding this comment.
According to the NMAquifer documentation, elevation accuracy is in units of feet. I don't think we need a separate units field here. We can either append "_ft" to the field name or just note the units in the documentation describing the field. I'm kind of inclined to do both.
There was a problem hiding this comment.
I generally prefer to avoid Hungarian Notation i.e. elevation_accuracy_m
There was a problem hiding this comment.
As long as it's documented somewhere that should be okay. If the units are always going to be the same we don't need a separate ..._unit table and can note it. Then in the response schema we can return a units field that evaluates to the correct unit.
Should the elevation_accuracy always be meters because elevation is stored in meters? And coordinate_accuracy should always be in decimal degrees because lat/long are stored in decimal degrees?
There was a problem hiding this comment.
From what I've read, best practice and industry standard is to store coordinate accuracy as a single, consistent linear unit, with meters being favored.
There was a problem hiding this comment.
Elevation in NMAquifer is stored in feet, so the elevation accuracy should also be stored in feet.
There was a problem hiding this comment.
In the transfer script we can convert elevation accuracy units from ft to meters
As for coordinate accuracy I'm okay to store that in meters, too, if that's the standard
schemas/location.py
Outdated
| elevation_accuracy: float | None = None | ||
| elevation_method: str | None = None | ||
| coordinate_accuracy: float | None = None | ||
| coordinate_accuracy_unit: str | None = None |
There was a problem hiding this comment.
If we decide to keep a coordiante_accuracy_unit field should this always be degree?
There was a problem hiding this comment.
The reason I added a units field for coordinates is because the NMAquifer lookup table, LU_CoordinateAccuracy, has units of second, minute, and meters. I thought a one-to-one mapping would be the fastest way to migrate the data, but we could convert coordinate accuracy units into a standard unit during the migration. From what I've read, it seems best practice and industry standard is to store coordinate accuracy as a single, consistent linear unit, with meters being favored.
Converting arc-seconds to linear units is a little tricky because the linear distance of a longitude
arc-second depends on the latitude, so lat/long will need to be a variable in the conversion. Just something to be aware of.
There was a problem hiding this comment.
I'll write a function for the second/minute conversion to meters and be sure to highlight it in the transfer PR for review. Hopefully Python has a function or library to do this already, but that may not be the case 🫰
| elevation_accuracy=row.AltitudeAccuracy, | ||
| elevation_method=row.AltitudeMethod, | ||
| # created_at=created_at, | ||
| # point_accuracy=row.CoordinateAccuracy, | ||
| # point_method=row.CoordinateMethod, | ||
| coordinate_accuracy=row.CoordinateAccuracy, | ||
| coordinate_method=row.CoordinateMethod, | ||
| nma_coordinate_notes=row.CoordinateNotes, | ||
| nma_notes_location=row.LocationNotes, |
There was a problem hiding this comment.
I can update this when I merge your work with the lexicon updates, but now that we are not using lookup table codes I think we'll need to amend this section (and for other tables/fields that are lexicon/lookup tables) to get the MEANING from the LU tables. Otherwise they won't be found in the lexicon term table and we'll run into errors.
On that note, I'll wait to open a PR for lexicon so that I can take care of that in the transfer scripts where it is relevant.
…odel, schema model, `test_location.py` file, and `conftest.py` file.
Codecov Report✅ All modified and coverable lines are covered by tests.
|
4 participants
Why
This PR addresses the following problem / context:
How
Implementation summary - the following was changed / added / removed:
util.pytransfer script file to include the new fields.Notes
Any special considerations, workarounds, or follow-up work to note?
state,county, andquad_namefields are auto-populated by the API.location.nameorthing.name