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

mpxv temporal colouring scale #57

Merged
merged 3 commits into from
Jun 20, 2022
Merged

Conversation

jameshadfield
Copy link
Member

Note: this requires forthcoming PRs in augur and auspice, and I'll notify here when released etc. Because of this i'll keep this as a draft PR.

The end result is
image

Note: this requires forthcoming PRs in augur and auspice
@jameshadfield
Copy link
Member Author

jameshadfield commented Jun 15, 2022

Augur PR: nextstrain/augur#969 (Update: done, released & image updated)
Auspice PR: nextstrain/auspice#1523 (Update: done, released & nextstrain.org updated)

@jameshadfield
Copy link
Member Author

jameshadfield commented Jun 16, 2022

Both of the prereq PRs above have been merged. Auspice is released (2.37.3) and the augur change should be released tomorrow 🤞

Update: The augur release will be 16.0, and this will require a few more things too:

  1. Wait until the nextstrain image has been updated (should be automatic)
  2. Add a check (in the Snakefile?) to ensure the user has >16.0

@victorlin
Copy link
Member

Wait until the nextstrain image has been updated (should be automatic)

This has been done: nextstrain/base:build-20220616T211939Z

This is necessary for the temporal colour scale introduced in the
preceding commit.

For searchibility i'll past the error message
that you would get when running with <16.0.0 (which shouldn't be
reachable as the snakemake pipeline should exit up front):

ERROR: 'temporal' is not one of ['continuous', 'ordinal', 'categorical', 'boolean']
Validation of config/auspice_config_mpxv.json failed.
@jameshadfield jameshadfield marked this pull request as ready for review June 17, 2022 01:54
@jameshadfield
Copy link
Member Author

Added a commit to ensure we're running augur v16, which is causing CI to fail as nextstrain/base:branch-nextalign-v2 has yet to be updated...

@corneliusroemer
Copy link
Member

I've restarted failing CI to see if the new image has propagated by now @jameshadfield

@corneliusroemer
Copy link
Member

After merging recent changes this now passes tests and should be mergable - but I leave the button pushing to you @jameshadfield :)

@corneliusroemer
Copy link
Member

I'll merge because it's better than what we use currently. But is it on purpose that there's no cake for the map?
image

@jameshadfield
Copy link
Member Author

jameshadfield commented Jun 20, 2022

but I leave the button pushing to you

Thanks for merging this -- it's the nice thing about working across different time zones 😂

But is it on purpose that there's no cake for the map?

I've not heard them called 🍰 but I like it. Continuous scales (and temporal is a continuous scale with small tweaks) get colour blending (e.g. https://next.nextstrain.org/zika/?c=num_date&d=map&p=full), however it's been a to-do to explore discretising these (e.g. via the binning in the legend) to produce a 🍰 . It would certainly make the map code simpler!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

3 participants