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
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions ingest/defaults/geolocation-rules.tsv
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,9 @@ Europe/Italy/Fvg/Gorizia Europe/Italy/Friuli Venezia Giulia/Gorizia
# Unclear which location is the real location
Europe/Netherlands/Utrecht/Rotterdam Europe/Netherlands//
North America/USA/Washington/Dc North America/USA/Washington DC/
# Following renamings as part of work on INRB Clade-I builds
Africa/Democratic Republic of the Congo/*/Nyagezi Africa/Democratic Republic of the Congo/*/Nyangezi
# 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.
Comment on lines +19 to +21
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.

Africa/Democratic Republic of the Congo/South Kivu/* Africa/Democratic Republic of the Congo/Sud Kivu/*