Add Chart Code export

[jwildfire]

Overview

This PR greatly simplifies the charting workflow in safetyGraphics by moving details from the charting module (mod_chartTab.R) and putting them in to the chart configuration object (makeChartConfig.R and friends). This change greatly simplifies the export workflow by allowing many components of the chart objects to be reused.

Details

See the posts below for a longer discussion of the approach used here. Here's a quick list of changes:

• Moved chart-level parameter management from mod_chartTab.R to the new makeChartParams.R. This allows for re-use in the export workflow.
• Added makeChartExport.R to create re-usable R scripts for charts/mappings.
• Added makeChartConfigFunctions.R to deal with binding functions to chart objects including the ui, server and main functions discussed below.
• The updates makeChartConfigFunctions.R allowed for a major simplification of mod_chartTab.R including the deletion of
  the mod_chartsRenderXXX.R family of functions.
• Replaced the reports tab with "Download html report" and "Download R script" buttons in the headers for each chart (implemented in mod_chartTab.R)
• Rewrote inst/report/safetyGraphicsReport.Rmd using the new framework. Biggest change is that the report only supports one chart (instead of multiple).

Fixes #518 #595

Test Notes

Checkout the branch and use the following script to initialize the app with the test modules and static plots included:

charts<-makeChartConfig()
charts[["rtf_render"]]$order<- -1
charts[["safetyOutlierExplorerModule"]]$order<-1
charts[["safetyOutlierExplorerStatic"]]$order<-1
safetyGraphicsApp(charts=charts)

Then test the following:

• Do a general review of the charting tab, and make sure that everything (filtering, mapping, etc) is working as expected.
• Export the html reports for each chart and confirm that they work as expected.
• Export the R script for each chart and confirm that running the script draws the chart. Note that no changes to the script are needed if you are using the default data, but the script needs updates when using custom data.

[jwildfire]

Adding notes from initial post and updating that with and overall summary.

---

Currently a work in progress, but so far the underlying charting process has been simplified as follows:

• a new makeChartParams() function has been added to avoid code duplication and standardize chart initialization workflow across chart types.
• The modules to render specific chart types (mod_chartsRenderXXX) have been removed. Now all rendering happens directly in mod_chartsTab. This should make it easier to standardize the rendering workflow across chart types ...

Next steps:

Ideally, I'd like to standardize chart rendering even further, possibly by extending the functions captured in the chart metadata. Each non-module chart needs the following functions:

• chart$workflow$ui called in chartsTabUI() (e.g.plotOutput())
• chart$workflow$server called in chartsTab() (e.g. renderPlot())
• chart$workflow$main that actually draws the chart called by the server function (although this is a bit different in widgets)

In an ideal world ,the charting tab module would look like this:

chartsTabUI <- function(id, chart){
  ns <- NS(id)    
  header<-makeChartSummary(chart)
  wrap<-chart$functions[[chart$workflow$ui]](ns("chart-wrap")) 
  return(list(header,wrap) 
}

chartsTab <- function(input, output, session, chart, data, mapping){  
  params <- reactive({ 
    makeChartParams(
      data = data(),
      mapping= mapping(),
      chart=chart
    )
   if(chart$type=="module"){
     callModule( chart$functions[[chart$workflow$server]], "chart-wrap", params())
   }else{
    output[["chart-wrap"]] <- renderChart(chart, params())
  }
  
  })
}

The problem is the renderChart is a little hard. Need to generalize this chunk:

   renderText(
      do.call(
        chart$functions[[chart$workflow$main]],
        params()
      )

I guess something like this might work:

do.call(
 chart$functions[[chart$workflow$render]],
      do.call(
        chart$functions[[chart$workflow$main]],
        params()
      )
)

But the nested do.call() doesn't seem the best idea. Maybe we can save that inner chunk as an object. Not sure. Will mess with it next week.

---

Making good progress here. The mod_chartTab workflow is now greatly simplified as described above, which has led to simplifications in the R markdown reporting as well. I'm strongly leaning towards only allowing single chart export in v2 using an export button from the chart header instead of a whole reports tab:

We can consider moving back to multi-chart export in a future release. Last big steps here are to work on code that allows users to reproduce charts, and then to clean up tests and checks.

[jwildfire]

OK - I think I'm done! Added a new makeChartExport() function to create scripts to re-run charts and fine tuned the report. Going to add some test notes to the initial post in the PR and get this out for review from @xni7 and @samussiah.

[jwildfire]

This fixes the bug noted in #595

[jwildfire]

@xni7 and @samussiah - just fixed a little bug that was preventing the test code from working correctly. Should be good to review now.

[xni7]

@xni7 and @samussiah - just fixed a little bug that was preventing the test code from working correctly. Should be good to review now.

@jwildfire , I am still having the "apply non-function" and grey out issue?

Warning: Error in chartsTabUI: attempt to apply non-function
  55: %>%
  54: module [/home/xni@avibio.com/projects/rpkgs/safetyGraphics/R/mod_safetyGraphicsServer.R#37]
  49: callModule
  48: server [/home/xni@avibio.com/projects/rpkgs/safetyGraphics/R/safetyGraphicsApp.R#34]
Error in chartsTabUI(id = ns(chart$name), chart = chart) : 
  attempt to apply non-function

[jwildfire]

@xni7 Did you use the custom chart object defined here?

charts<-makeChartConfig()
charts[["rtf_render"]]$order<- -1
charts[["safetyOutlierExplorerModule"]]$order<-1
charts[["safetyOutlierExplorerStatic"]]$order<-1
safetyGraphicsApp(charts=charts)

This runs ok for me now, but it will bomb if you leave in the RTF renderer. Might try to fix that today ...

[xni7]

Yeah it works without the RTF!

[jwildfire]

Updated the rtf renderer here, and updated the test notes in this PR accordingly.

[xni7]

App and code look amazing! checked most charts, all are downloadable. chart is reproducible with the code. Also tested with custom data, it also worked. Just a few minor comments

• Some widgets' dimension might need adjustment, e.g.
  
• It would be desirable to capture user's input data in the html report?
• In the "details" tab code box, it would be desirable to have a button to copy the code to clipboard.

[jwildfire]

Thanks @xni7! Going to go ahead and merge this in a bit. A few notes on your comments below. Feel free to file issues if you think they're needed :)

It would be desirable to capture user's input data in the html report?

You mean rendering a table? That's a good idea ... I'll add an issue. Would love to improve the data-handling in the R script as well, but not sure the best way to do that.

Some widgets' dimension might need adjustment

This is very likely an issue for the js renderer. Might be able to figure outa work-around, but probably going to leave this until after v2 since it's a safetyCharts tweak.

In the "details" tab code box, it would be desirable to have a button to copy the code to clipboard.

This is pretty easy. I'll file an issue.