nextstrain / augur

Pipeline components for real-time phylodynamic analysis
https://docs.nextstrain.org/projects/augur/
GNU Affero General Public License v3.0
268 stars 128 forks source link

ENH: Add `augur colors` subcommand based on `assign-colors.py` scripts #1185

Open corneliusroemer opened 1 year ago

corneliusroemer commented 1 year ago

We discussed adding more reusable components to augur at our meeting yesterday.

Working on ncov PR https://github.com/nextstrain/ncov/pull/1050 I encountered some limitations of the assign-colors.py script. This script could benefit from new features.

The script is used in a lot of places, it could be a prime candidate for augurification: see where it is used https://github.com/search?type=code&auto_enroll=true&q=assign-colors.py

The ncov version is here: https://github.com/nextstrain/ncov/blob/master/scripts/assign-colors.py

Some features to possibly add to the existing script:

corneliusroemer commented 1 year ago

Just rediscovered that augur export v2 doesn't accept color_ordering.tsv despite this being a pervasive config file pattern for current workflows. Another argument for augur colors

huddlej commented 1 year ago

I just reinvented the wheel a bit in the seasonal flu repo with a similar command to generate a custom color scale in an Auspice config JSON. Thanks to @joverlee521 for reminding me that this is a more general need across pathogen workflows!

I can see how it's useful to get different output types like an Auspice config JSON color scale or a colors TSV file, depending on what you're trying to do. It's also useful to limit the values that get assigned colors to a subset of what appears in a given analysis. One could imagine supporting both an opt-in approach like the script I linked to above where the user specifies the values that should get colors and an opt-out approach where the user specifies the values that should not get assigned colors.

joverlee521 commented 1 year ago

During discussion today, we floated the idea of vendoring the color script similar to how we vendor nextstrain/ingest scripts since assign-colors.py feels a little too ad-hoc to officially release as an augur command.

victorlin commented 1 week ago

Copying from https://github.com/nextstrain/avian-flu/pull/102#discussion_r1826096664 for visibility.

Additional modification to consider:

- line.lstrip().rstrip()
+ line.strip()