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

multi region/indicator time series #1136

Open
wants to merge 11 commits into
base: dev
Choose a base branch
from
Open

Conversation

sgratzl
Copy link
Member

@sgratzl sgratzl commented Jan 18, 2022

closes #1113

Prerequisites:

  • Unless it is a hotfix it should be merged against the dev branch
  • Branch is up-to-date with the branch to be merged with, i.e. dev
  • Build is successful
  • Code is cleaned up and formatted

Summary

two new widgets for the dashboard builder: multi region and multi indicator time series:
depending on the selection it will be a single or dual axis:

image

@sgratzl sgratzl added the enhancement New feature or request label Jan 18, 2022
@sgratzl sgratzl self-assigned this Jan 18, 2022
@netlify
Copy link

netlify bot commented Jan 18, 2022

✔️ Preview link ready!

🔨 Explore the source changes: 832ac51

🔍 Inspect the deploy log: https://app.netlify.com/sites/cmu-delphi-covidcast/deploys/61e6d2012b78a1000744afd0

😎 Browse the preview: https://deploy-preview-1136--cmu-delphi-covidcast.netlify.app

@krivard krivard requested a review from ryantibs December 7, 2022 15:50
@ryantibs
Copy link
Member

ryantibs commented Dec 8, 2022

Thanks for tagging me, I had somehow not seen this when it first arrived.

I think it's a great addition and will be very useful. My thoughts below.

Main comment: it seems like we are limiting ourselves to 2 curves on either the multi-indicator or multi-region views (I tried selecting more than 2 but only 2 show up). I would like to allow 3 or more in both multi-indicator and multi-region modes. Let me explain.

  • For multi-region, we should just have a single y-axis (not dual y-axis) since all signals will be of the same units anyway. If they're not comparable across geographies then that's why we have proportion versions (rates instead of counts). So I think it's not a main use case to want to be able to have separate axes for the same indicator over multiple regions. And we could simplify to a single y-axis, and then you should be able to add as many regions as you want to the plot.

  • For multi-indicator, if the selection is of size 2, then I think it's nice to allow a dual y-axis. But you should be allowed to select more than 2, and then we just get a single axis and everything gets put onto an arbitrary scale from (say) 0 to 100, where for each signal, we scale it so that the min value over the selected time range being is 0, and so that the max value over the time range is 100. Then when you hover, you could see the actual values in original units for each signal.

Minor comment (unrelated to his PR, but it bugs me especially in the context of multiple signals): I would like us to remove "Rescale Y-Axis" radio button in general, because I think it's seldomly used (it doesn't appear anywhere else on our dashboards) and in the multi-signal context I find it confusing. I would expect rescaling to mean something else here.

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

Successfully merging this pull request may close these issues.

Enable signal comparisons on time series plots
2 participants