Closed francisbarton closed 9 months ago
This is great, thanks Fran! Particular nod to the effort to untangle conflicts following recent dev work.
I'd like to give it a test - drive before merging to understand how to use the functionality. In doing that I'll also understand enough to write a vignette covering theory and usage.
One question, what is partial about the change?
Designed to partially address issue #4
I think it's partial in two ways:
I really appreciate this contribution Fran. I'm going to pause other development until we find a way to get this merged.
Quick review:
[x] Ideally need to mark to final point on a rare-event chart to reflect that it is a provisional point (ie. this is what the chart would look like if we had an event today). For simplicity, the other option is to not draw the final point. Users may get used to this in time, and with explanation.
[x] Potential for confusion with two different-looking charts with the same measure ref and title. The Y axis is different, but we could consider eg. appending "(time between)" to the measure title for clarity. This would make the rolled up view in particular clearer.
[ ] Tests (light touch - what do we need to prevent accidental regression later)
[ ] Does the "none" sheet work when there are multiple rare-event measures, with different event dates in columns? We probably need to demo that for users by adding a 2nd rare event measure to this sheet to show how it might look.
[x] Docs need updating to read in the "none" worksheet of data.xlsx as part of constructing the measure_data
object.
[ ] Vignette covering the how and why of rare-event charts, and referring to the Health Care Data Guide for further reading.
[ ] Add commentary to the measure 99 charts explaining what they are in the example report. We could take this further and have similar commentary for each successive chart, each introducing a new concept (1 = basic plot, 2 = target line, 3 = improvement direction = increase, 4 = improvement_direction = decrease, 5 = rebase, 6 = rare events on an XmR chart, 7 = rare events on a T chart, etc).
I love:
Possible changes after initial merge:
I'm merging this to a temporary dev branch, because I want to change the API slightly.
I'd like rare events to be supplied as an event list, and for the processing to be done inside this package. This is in keeping with the method used by the making data count excel tools.
I will open issues for the rare events vignette, and the improvement to the commentary for each chart in the example report.
If the rare event chart flag is set to "Y" in the report_config and there is calculated "days between" data in the "none" sheet of the measure data file, a "days between" chart should be created.
All tests and package checks pass.
Designed to partially address issue #4