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 lat/longs #1449

Merged
merged 1 commit into from
May 9, 2024
Merged

Update lat/longs #1449

merged 1 commit into from
May 9, 2024

Conversation

trvrb
Copy link
Member

@trvrb trvrb commented Apr 24, 2024

This commit pulls in all the heavily curated lat/long values from https://github.com/nextstrain/ncov/blob/master/defaults/lat_longs.tsv. These should have gotten a lot of eyes on them from daily ncov curation work and I'd hope that they'd just work without additional testing today.

I've found a large number of occasions where running analyses for other pathogens (measles, avian flu, etc...) we're dropping lat/longs with just warnings because they don't exist in the data. I think better to pad this out in Augur than to force all the individual pathogen repos to copy out a long list of lat/long locations.

If a pathogen repo wants to override, this is very possible.

This commit pulls in all the heavily curated lat/long values from https://github.com/nextstrain/ncov/blob/master/defaults/lat_longs.tsv. These should have gotten a lot of eyes on them from daily ncov curation work and I'd hope that they'd just work without additional testing today.

I've found a large number of occasions where running analyses for other pathogens (measles, avian flu, etc...) we're dropping lat/longs with just warnings because they don't exist in the data. I think better to pad this out in Augur than to force all the individual pathogen repos to copy out a long list of lat/long locations.

If a pathogen repo wants to override, this is very possible.
@trvrb trvrb requested a review from a team April 24, 2024 00:02
Copy link
Member

@victorlin victorlin left a comment

Choose a reason for hiding this comment

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

This doesn't affect workflows which use custom --lat-longs, which seems to be most of the currently maintained core workflows. Might be a good to create a higher level issue to track transition of pathogen workflows away from custom --lat-longs to use the improved default from this PR.

Failing CI in avian-flu is irrelevant, I'll address separately.

@trvrb
Copy link
Member Author

trvrb commented May 9, 2024

Thanks @victorlin! You're right that it would be nice to slim down lat_longs.tsv files in individual repos. But at least getting this into Augur should reduce the amount that future lat_longs.tsv files need to be populated.

@trvrb trvrb merged commit 5d04da2 into master May 9, 2024
19 of 20 checks passed
@trvrb trvrb deleted the update-lat-longs branch May 9, 2024 23:10
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