Timeseries plot and signaltab UI

[tomasmartbert]

Resolves #231, #277 and #279.

Most of this PR focuses on fixing various issues related to the time series plot, plus some minor stuff such as spelling errors and code refactoring in the module-code for the signal tab.

The time series barplot now has bars centered mid-week such that the threshold line no longer appears to step up/down in the middle of the week. The blue background of the signal detection period has been expanded to cover the bars accordingly.

Various bugs have been fixed by being more explicit about plot ranges. Some JavaScript is injected to solve plotly-quirks with the interactive plot when zooming in on specific date ranges.

Note: htmlwidgets is a new dependency.

Discussion points

231: The plotly range selector buttons in the top-left corner of the interactive plot only adjusts on the start date of the displayed date-interval - the end date is left as-is - which means, that when first panning the slider and then pressing a range selection button, e.g. 6 months as in the figure below, the end date displayed in the zoomed plot is no longer the last date in data.

Should our design intention be to always get the right interval end point to the most current date when pressing one of the range selector buttons? That might not be possible with plotly as-is, but is likely possible by extending the JavaScript in this PR.

277: Two remaining issues:

• The plotly dates on the x-axis are still Sundays (hard to change how plotly decides).
• The hover dates are still Mondays (from the results object in plot_time_series()) - Should we change it to display <year>-W<week>?

Adaptive y-axis

A local ymax for each week is now calculated taking into account the max of the case numbers, threshold line or signal marker. The overall ymax is used for clipping the y-range on the plotly-figure and is set to 1,2,5 * b (b a power of 10). As it is now the, sliderange inset at the bottom of the figure is using the same ymax which may cause the threshold line to not be visible on the inset. See example here:

- Should we opt for the signal threshold line is always visible on the sliderange inset? If always visible it is easier to relate the zoomed date range earlier in the period to the calculated threshold levels later on, but it might be more difficult to discern smalller case numbers.

[tomasmartbert]

I'll commit an update in two hours addressing the last discussion point and readjustment of the ymax value.

[tomasmartbert]

The computation of the ymax for the y-axis range sometimes leads to too much wasted vertical space. E.g. if the max data-value is just above 20 it triggers the custom_round_up() function to set the y-axis range to go to 50, as in the figure below where the signal marker is set at y=19*1.1 = 20.9 which is then transformed by custom_round_up() to a ymax=50. I have now made a simplified custom_round_up() to remedy this issue:

[tinneuro]

Thanks @tomasmartbert for this big amount of work! Takes time to review all of that.

Should we change it to display -W?

I would say yes, as anyways it is not about the exact day and might be confusing and is clearer when the week is shown.

Should we opt for the signal threshold line is always visible on the sliderange inset?

Is this solved with your latest update? If not I would say yes it is nice but if difficult not necessary.

Should our design intention be to always get the right interval end point to the most current date when pressing one of the range selector buttons?

I don't think it is necessary. For me it is also fine that it takes the last "end time point" selected by the user like it does right now and then extends it. The only part which is not good is that the 'Signal Detection Period' does not work as intended in this approach. I think we should either remove this button or try that for this button the window is really set to the Signal Detection Period.

[tinneuro]

For my understanding:

The plotly dates on the x-axis are still Sundays (hard to change how plotly decides

Due to this the ticks are not at the same position as the bars, right? I don't like it but it seems difficult to fix this...

[tinneuro]

Did you consider reporting the bug of plotly extending into the future to https://github.com/plotly/plotly.R/issues ?

[tomasmartbert]

Thanks @tomasmartbert for this big amount of work! Takes time to review all of that.

Thank you @tinneuro, sure it takes time to follow it through :)

Should we change it to display -W?

I would say yes, as anyways it is not about the exact day and might be confusing and is clearer when the week is shown.

I agree, I'll see if I can fix it at some point soon, otherwise in some other issue.

Should we opt for the signal threshold line is always visible on the sliderange inset?

Is this solved with your latest update? If not I would say yes it is nice but if difficult not necessary.

It is solved in the latest update.

Should our design intention be to always get the right interval end point to the most current date when pressing one of the range selector buttons?

I don't think it is necessary. For me it is also fine that it takes the last "end time point" selected by the user like it does right now and then extends it. The only part which is not good is that the 'Signal Detection Period' does not work as intended in this approach. I think we should either remove this button or try that for this button the window is really set to the Signal Detection Period.

I don't know how easy it is capture and handle those button-click events. Removing the button would probably not loose a lot, since the SDP is also shown quite clearly by the lightblue background.

[tomasmartbert]

For my understanding:

The plotly dates on the x-axis are still Sundays (hard to change how plotly decides

Due to this the ticks are not at the same position as the bars, right? I don't like it but it seems difficult to fix this...

Exactly! If plotly could be made Monday-based, the ticks would be where the threshold lines step up/down, and the left side of the bars would be just after each tick (there is some margin space on the left and right side of each bar).

[tomasmartbert]

Did you consider reporting the bug of plotly extending into the future to https://github.com/plotly/plotly.R/issues ?

Not really, I found a lot of posts where people have the same complains, so I guessed (but don't know) it has been reported. I'll have a look, thanks! [UPDATE] https://github.com/plotly/plotly.js/issues/6995

[tinneuro]

I don't know how easy it is capture and handle those button-click events. Removing the button would probably not loose a lot, since the SDP is also shown quite clearly by the lightblue background.

Yes let's go for now by removing it. Do you think you can do this today to remove it?

I agree, I'll see if I can fix it at some point soon, otherwise in some other issue.

As I want to get things merged soon to finally produce the next release let's say if you can manage to integrate it until tomorrow lunchtime we do it in this PR otherwise a new issue.

Furthermore I just realised that when using age groups as stratification in the report the timeseries plots by age group are no longer there. For county for example they are still appearing. This already happened at an earlier stage as in the current develop we have the same problem.

[tomasmartbert]

I don't know how easy it is capture and handle those button-click events. Removing the button would probably not loose a lot, since the SDP is also shown quite clearly by the lightblue background.

Yes let's go for now by removing it. Do you think you can do this today to remove it?

I can do that now.

I agree, I'll see if I can fix it at some point soon, otherwise in some other issue.

As I want to get things merged soon to finally produce the next release let's say if you can manage to integrate it until tomorrow lunchtime we do it in this PR otherwise a new issue.

Fine with me - I'll spend a little time today on it.

Furthermore I just realised that when using age groups as stratification in the report the timeseries plots by age group are no longer there. For county for example they are still appearing. This already happened at an earlier stage as in the current develop we have the same problem.

Now you mention it, I also experienced that, but it is unlikely related to the time_series_plot() function or signal tab. I'm happy to take a quick look into it, but otherwise I think we should file a bug issue.

[tinneuro]

Otherwise you do the first two points and I can try to take a look into the last problem with the age group stratifications not appearing anymore

[tinneuro]

I will try to finish my side tomorrow morning. Decided to make some further changes to help the debugging process.

[tomasmartbert]

I will try to finish my side tomorrow morning. Decided to make some further changes to help the debugging process.

OK, I have just pushed some commits - I'm off tomorrow - good luck :)