Skip to content

Module renderer#440

Merged
jwildfire merged 19 commits intodevfrom
module-renderer
Dec 14, 2020
Merged

Module renderer#440
jwildfire merged 19 commits intodevfrom
module-renderer

Conversation

@jwildfire
Copy link
Contributor

@jwildfire jwildfire commented Nov 6, 2020

Overview

This branch refactors the process for adding custom charts and reimplements shiny module renderers. See the draft of the new Custom Chart Vignette for details about the new process.

Details

The biggest change is that chart configuration information has been migrated from raw-data/charts.csv to a series of yaml files. The yaml files are currently saved in inst/config/charts, but will be migrated to the safetyCharts package in the near future. Detailed documentation of the YAML format is saved in this wiki page, which will be adapted in to a vignette before the v2 release.

Test notes

Checkout the branch and then run this code to start the app.

devtools::install_github("safetyGraphics/safetyCharts")
library(safetyCharts)
devtools::install()
library(safetyGraphics)
safetyGraphics::safetyGraphicsApp()

Just load the package and run safetyGraphicsApp() like normal. You can also run makeChartConfig() directly to easily create a list of charts from the YAML files saved in a set of directories.

@jwildfire jwildfire marked this pull request as draft November 6, 2020 18:54
@xni7
Copy link
Contributor

xni7 commented Nov 21, 2020

Test notes

Checkout the branch and then run this code to start the app.

devtools::install_github("safetyGraphics/safetyCharts") 
library(safetyCharts)
devtools::install()
library(safetyGraphics)
safetyGraphics::safetyGraphicsApp()

@jwildfire I had issue installing devtools::install_github("safetyGraphics/safetyCharts"), it complained "object 'nep_explorer_data' not found". There is some test code in R_old that requires this test data but not included. Was able to install a local branch after removing "R_old"

Co-authored-by: Xiao Ni <mooncase@gmail.com>
@jwildfire
Copy link
Contributor Author

jwildfire commented Nov 23, 2020

There is some test code in R_old that requires this test data but not included.R_old that requires this test data but not included

@xni7 I just added R_Old to .rbuildignore in safetycharts. I'm able to build ok here, but wasn't getting that bug before either, so let me know if that fixes the issue on your end.

@xni7
Copy link
Contributor

xni7 commented Nov 24, 2020

There is some test code in R_old that requires this test data but not included.R_old that requires this test data but not included

@xni7 I just added R_Old to .rbuildignore in safetycharts. I'm able to build ok here, but wasn't getting that bug before either, so let me know if that fixes the issue on your end.

it installs okay now

@xni7
Copy link
Contributor

xni7 commented Nov 24, 2020

@jwildfire , couldn't get Example 1 - Static Outlier Explorer working. Seems to have do with the ggplot aes mapping.
Also where to load user supplied custom data? Is this implemented yet?

image

@jwildfire
Copy link
Contributor Author

@jwildfire , couldn't get Example 1 - Static Outlier Explorer working. Seems to have do with the ggplot aes mapping.

Hm, seems like a data issue ... Is there any detail in the R log? I'm going to fix the data preview section now so that this will be easier to debug.

@jwildfire
Copy link
Contributor Author

Also where to load user supplied custom data? Is this implemented yet?

Right now, the workflow is to load your data in R and then pass it in to the safetyGraphics app like so:

mylabsdata <- read.csv(...)

safetyGraphicsApp(data=list(labs=myLabdata, ...)

Eventually we want to create a helper app/plugin/widget that allows non-technical users to load data (and apply other customizations) via a simple GUI. This will likely look at least a little bit like Bo's shinymeta workflow. This is tracked in #427 so we can discuss details there.

@jwildfire
Copy link
Contributor Author

I'm going to fix the data preview section now so that this will be easier to debug.

Updated now. Can you check your labs data and confirm that it is showing up as expected and has a column called "PARAM"

@MayaGans
Copy link
Contributor

You can also run makeChartsSettings() directly to easily create a list of charts from the YAML files saved in a set of directories.

I was able to open the app (woohoo!) and I wanted to try this alternative you posed, @jwildfire but I don't see this function anywhere when I look using safetyGraphics:::* ?

@jwildfire
Copy link
Contributor Author

I don't see this function anywhere when I look using safetyGraphics:::* ?

@MayaGans It was a typo. I meant makeChartConfig. I'll edit the original message to fix in a sec. The workflow I was alluding was to call makeChartConfig() directly and then pass the results to safetyGraphicsApp() like this:

customDirectories<- c("some/custom/charts", "safetyCharts/inst/config")
customCharts<-makeChartConfig(dirs=customDirectories)
safetyGraphicsApp(charts=customCharts)

@xni7 this is the current workflow for adding custom charts to the defaults found in safetyCharts.

@jwildfire jwildfire marked this pull request as ready for review December 11, 2020 17:10
@jwildfire
Copy link
Contributor Author

jwildfire commented Dec 11, 2020

Merging after discussion with technical team - Still plenty of work to do, but everyone is generally able to run the app locally. Onward!

Close #429 #431 #438 #442 #450

@jwildfire jwildfire closed this Dec 11, 2020
@jwildfire jwildfire reopened this Dec 14, 2020
@jwildfire
Copy link
Contributor Author

Oops - merging before closing ...

@jwildfire jwildfire merged commit 81741eb into dev Dec 14, 2020
@jwildfire jwildfire deleted the module-renderer branch December 14, 2020 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants