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

export: Add option to extend the default lat/longs #1465

Open
victorlin opened this issue May 16, 2024 · 3 comments
Open

export: Add option to extend the default lat/longs #1465

victorlin opened this issue May 16, 2024 · 3 comments
Labels
enhancement New feature or request

Comments

@victorlin
Copy link
Member

victorlin commented May 16, 2024

Context

Currently, --lat-longs overrides the default mappings provided by augur/data/lat_longs.tsv.

Description

Add an option to extend the default mappings in addition to the existing behavior of replacing them.

For entries in both, the extending file should take precedence over the default.

Why?

The current behavior has encouraged curation of lat/long values independently across pathogen repos, rather than in a centralized place. Due to increased attention during the SARS-CoV-2 pandemic, the ncov workflow has a carefully curated lat/longs file. A recent PR #1449 moved all that effort into Augur's default mapping, however at this point a significant amount of pathogen repos already have their own lat/longs files.

Proposed solutions

These are based on my speculation that most pathogen repos would benefit from extending rather than overriding – I could be wrong.

1. Switch behavior directly

  1. Make --lat-longs extend instead of replace defaults in a major release.

2. Switch behavior with deprecation

  1. Add an option --lat-longs-extend with this new additive behavior.
  2. Link the current option --lat-longs to a new name --lat-longs-override with a deprecation warning that the behavior of --lat-longs will switch to --lat-longs-extend in the future.
  3. Switch --lat-longs behavior to --lat-longs-extend in a major version release.
@victorlin victorlin added the enhancement New feature or request label May 16, 2024
@j23414
Copy link
Contributor

j23414 commented May 16, 2024

+ 1 for most pathogen repos would benefit from extending rather than overriding

@jameshadfield
Copy link
Member

+1 for Appending user-provided data onto the defaults. This is how --colors works.

What's the downside to switching the behaviour of --lat-longs in a single step? For a repo with their own lat-longs file then there will be no change unless they are missing some which will now be filled in by the defaults, and the only reason this wouldn't be desirable is if they are using missing lat-longs as a hack to avoid plotting certain demes on the map. I'd suggest changing the behaviour of --lat-longs now (with a major version release).

@victorlin
Copy link
Member Author

@jameshadfield good idea, I've added that as the first solution in the issue description.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants