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

Update geo-fixes #279

Merged
merged 1 commit into from
Sep 27, 2024
Merged

Update geo-fixes #279

merged 1 commit into from
Sep 27, 2024

Conversation

jameshadfield
Copy link
Member

See added comments in TSV

  • Checks pass

See added comments in TSV
@jameshadfield jameshadfield mentioned this pull request Sep 27, 2024
@jameshadfield
Copy link
Member Author

I'm going to merge this ASAP (post-merge review welcome) as none of our mpox builds currently have sub-country level metadata included. #278 will add this to the Clade-I build, and we are using it for INRB builds as well.

@jameshadfield jameshadfield reopened this Sep 27, 2024
@jameshadfield jameshadfield merged commit b453693 into master Sep 27, 2024
5 of 6 checks passed
@jameshadfield jameshadfield deleted the james/geo-fixes branch September 27, 2024 01:10
Comment on lines +19 to +21
# Note that in our master geolocation rules we replace "Sud-Kivu" with "Sud Kivu" (similarly for "Nord-Kivu")
# however the INRB tends to use "Sud-Kivu". We can't add a rule here to change it back (`augur curate` doesn't
# allow it) so we enforce the use of "Sud Kivu" in INRB data as well.
Copy link
Contributor

Choose a reason for hiding this comment

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

I was curious what you meant by "augur curate doesn't allow it"...Because we just concatenate these to the "general" geolocation rules, the conflicting rules raises the CyclicGeolocationRulesError in augur curate apply-geolocation-rules.

Not sure if this is something we should handle within Augur or if the workflow should be improved to not blindly concatenate the geolocation rules...

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure if this is something we should handle within Augur...

If we're going to continue with the pattern of a master-list plus some repo-specific additions it'd be nice if those additions could function as overrides. I.e. if the master list replaces X with Y, we could add an override of Y to X (or Y to Z etc) in a repo-specific manner.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, that makes sense! Tracking in nextstrain/augur#1648.

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.

2 participants