search

epiforecasts / RtD3

Rt visualisation in D3
https://epiforecasts.io/RtD3/
Other
6 stars 2 forks source link

Make Region default and change summaryData #10

Closed hamishgibbs closed 4 years ago

hamishgibbs commented 4 years ago

Had a bit of confusion about the correct branch to push to...

But I have brought your changes here and changed rt_vis to accept Region/region as defaults, rather than Country/country.

Also - you are correct that summaryData should be included in the rtData list - not as a separate file like geoData.

Now, all rtData must contain a summaryData field.

Also - the map will now update when the data source is toggled (i.e. 'Cases' or 'Deaths') with the new summary data displayed.

Docs still need updating but could you check over these changes before I merge? It may change your current implementation unless you are using readInEpiNow2 which I believe should work the same in this new format, perhaps region_var will need to be changed appropriately.

hamishgibbs commented 4 years ago

@seabbs

seabbs commented 4 years ago

So the standard approach would have been to accept my PR and then do another PR with these changes but all good :).

Sounds like a sensible update for summaryData.

It looks like this PR is missing a fix I pushed to the other one earlier (arrgh git is complicated!)

So implementation should all work as using readInEpinow2 but I'll check when I next update.

hamishgibbs commented 4 years ago

Apologies for the confusion - I will go look and add the fix from the previous branch.