Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BDRSPS-918] First stab adding coordinateUncertaintyInMeters #290

Merged
merged 12 commits into from
Nov 6, 2024

Conversation

chungvl
Copy link
Contributor

@chungvl chungvl commented Oct 30, 2024

No description provided.

@chungvl chungvl marked this pull request as ready for review October 31, 2024 02:32
@chungvl chungvl changed the title [BDRSPS-918] First stab at ticket [BDRSPS-918] First stab adding coordinateUncertaintyInMetres Oct 31, 2024
@chungvl chungvl changed the title [BDRSPS-918] First stab adding coordinateUncertaintyInMetres [BDRSPS-918] First stab adding coordinateUncertaintyInMeters Oct 31, 2024
Copy link
Collaborator

@joecrowleygaia joecrowleygaia left a comment

Choose a reason for hiding this comment

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

I'm not sure that it is required for metadata template since the field doesn't exist.

abis_mapping/templates/survey_metadata/mapping.py Outdated Show resolved Hide resolved
abis_mapping/templates/survey_metadata/mapping.py Outdated Show resolved Hide resolved
@chungvl chungvl changed the title [BDRSPS-918] First stab adding coordinateUncertaintyInMeters Draft: [BDRSPS-918] First stab adding coordinateUncertaintyInMeters Nov 5, 2024
@chungvl chungvl changed the title Draft: [BDRSPS-918] First stab adding coordinateUncertaintyInMeters [BDRSPS-918] First stab adding coordinateUncertaintyInMeters Nov 5, 2024
@Lincoln-GR
Copy link
Contributor

Lincoln-GR commented Nov 6, 2024

@chungvl Can you try running the poe generate-example-ttl-files script and committing the result to this PR.
This should greatly reduce the overall diff of the .ttl files to make them easier to review, and surface the actual changes you are making.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this means there was an upgrade applied to the poetry project, or the lockfile was deleted and recreated. Does it need to be left as-was @serge-gaia or @Lincoln-GR ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like the sub-dependencies were bumped. This will only effect development and CI on this project, not part-a.
Should be fine to leave it. Or on the other hand no reason it needs to be in this PR 🤷

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Prior to doing an update the generate-example-ttl script wasn't working for me.

Copy link
Contributor

Choose a reason for hiding this comment

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

For next time, I would imagine just doing poetry install to install the current dependencies, should be sufficient to get the script working without changing the lock file.
(Assuming there isn't an actual incompatibility between our code and a dependency)

@joecrowleygaia joecrowleygaia merged commit 885ecf0 into main Nov 6, 2024
7 checks passed
@chungvl chungvl deleted the BDRSPS-918-a branch November 7, 2024 03:32
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