Skip to content

Support multiple libPaths#494

Merged
jwildfire merged 10 commits intodevfrom
fix-441
Apr 13, 2021
Merged

Support multiple libPaths#494
jwildfire merged 10 commits intodevfrom
fix-441

Conversation

@jwildfire
Copy link
Contributor

Overview

Iteratively looks for safetyCharts across multiple libPaths to fix #441.

Test Code

install.packages("safetyData")
library(safetyData)
devtools::install_github("safetyGraphics/safetyCharts", ref="dev")
library(safetyCharts)
devtools::install()
library(safetyGraphics)
safetyGraphics::safetyGraphicsApp()

@jwildfire jwildfire requested review from samussiah and xni7 February 10, 2021 15:17
@jwildfire jwildfire self-assigned this Feb 10, 2021
@jwildfire jwildfire changed the base branch from gh-actions to dev February 10, 2021 18:27
@samussiah
Copy link
Contributor

Hitting a few errors:

> safetyGraphicsApp()
Found 14 config files: aeExplorer, aeTimelines, hepexplorer, labdist, safetyHistogram, safetyOutlierExplorer, safetyOutlierExplorerModule, safetyOutlierExplorerStatic, safetyResultsOverTime, safetyResultsOverTimeStatic, safetyShiftPlot, tendril, tplyr_demog, tplyr_shift
Global Functions: aeExplorer_initlabdist_serverlabdist_uisafety_histogram_charttplyr_aes_charttplyr_shift_chart
+ aeExplorer: Found 1 of 1 workflow functions, and 0 other functions.
+ aeTimelines: Found 0 of 0 workflow functions, and 0 other functions.
+ hepexplorer: Found 0 of 0 workflow functions, and 0 other functions.
+ labdist: Found 2 of 2 workflow functions, and 0 other functions.
+ safetyHistogram: Found 0 of 0 workflow functions, and 0 other functions.
+ safetyOutlierExplorer: Found 1 of 1 workflow functions, and 0 other functions.
+ safetyOutlierExplorerModule: Found 2 of 2 workflow functions, and 0 other functions.
+ safetyOutlierExplorerStatic: Found 1 of 1 workflow functions, and 0 other functions.
+ safetyResultsOverTime: Found 1 of 1 workflow functions, and 0 other functions.
+ safetyResultsOverTimeStatic: Found 1 of 1 workflow functions, and 0 other functions.
+ safetyShiftPlot: Found 1 of 1 workflow functions, and 0 other functions.
+ tendril: Found 1 of 1 workflow functions, and 0 other functions.
+ tplyr_demog: Found 1 of 1 workflow functions, and 0 other functions.
+ tplyr_shift: Found 1 of 1 workflow functions, and 0 other functions.
Loading required package: shiny

Listening on http://127.0.0.1:4268
chartsTab() starting for aeExplorer
chartsTab() is initializing a widget at aeExplorer-wrap
chart is aeExplorer; package is safetyCharts
chartRenderWidget() starting for aeExplorer
chartsTab() starting for aeTimelines
chartsTab() is initializing a widget at aeTimelines-wrap
chart is aeTimelines; package is safetyCharts
chartRenderWidget() starting for aeTimelines
chartsTab() starting for hepexplorer
chartsTab() is initializing a widget at hepexplorer-wrap
chart is hepexplorer; package is safetyCharts
chartRenderWidget() starting for hepexplorer
chartsTab() starting for labdist
chartsTab() is initializing a module at labdist-wrap
chartsTab() starting for safetyHistogram
chartsTab() is initializing a widget at safetyHistogram-wrap
chart is safetyHistogram; package is safetyCharts
chartRenderWidget() starting for safetyHistogram
chartsTab() starting for safetyOutlierExplorer
chartsTab() is initializing a widget at safetyOutlierExplorer-wrap
chart is safetyOutlierExplorer; package is safetyCharts
chartRenderWidget() starting for safetyOutlierExplorer
chartsTab() starting for safetyOutlierExplorerModule
chartsTab() is initializing a module at safetyOutlierExplorerModule-wrap
chartsTab() starting for safetyOutlierExplorerStatic
chartsTab() starting for safetyResultsOverTime
chartsTab() is initializing a widget at safetyResultsOverTime-wrap
chart is safetyResultsOverTime; package is safetyCharts
chartRenderWidget() starting for safetyResultsOverTime
chartsTab() starting for safetyResultsOverTimeStatic
chartsTab() starting for safetyShiftPlot
chartsTab() is initializing a widget at safetyShiftPlot-wrap
chart is safetyShiftPlot; package is safetyCharts
chartRenderWidget() starting for safetyShiftPlot
chartsTab() starting for tendril
chartsTab() starting for tplyr_demog
chartsTab() starting for tplyr_shift
Warning: Error in : Only strings can be converted to symbols
  96: <Anonymous>
Warning: Error in : Only strings can be converted to symbols
  50: <Anonymous>
Warning: Error in : Only strings can be converted to symbols
  81: <Anonymous>
Warning: Error in : Only strings can be converted to symbols
  65: <Anonymous>

dirs<-paste(.libPaths(),'safetycharts','config', sep="/")
for(lib in .libPaths()){
dirs<-paste(lib,'safetyCharts','config', sep="/")
if(file.exists(dirs)) break
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So basically choose the first libPath. This seems to do the trick.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would it be useful to add an error msg after the loop if dirs are not found stop('no chart config found, please install safetyCharts first'). If we force safetyCharts as a dependency pkg then it won't be an issue.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point @xni7, it's not immediately obvious to me how safetyCharts fits into the workflow when using a custom set of chart config files. It seems the workflow functions are pulled both from the location of the chart config files as well as from the safetyCharts package.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, you definitely can run safetyGraphics without safetyCharts if you supply all your own renderers and config files, but I'm not sure how often that will actually happen.

For this chunk of code, safetyCharts is required unless the dirs option is set. So I think a warning is totally reasonable. I'll add something now and then merge.

Copy link
Contributor

@samussiah samussiah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The errors I hit previously have been fixed but I'm hitting new errors in a four anonymous functions: Warning: Error in : Only strings can be converted to symbols. The full output is in a comment above.

@samussiah samussiah requested a review from xni7 March 9, 2021 21:37
@samussiah
Copy link
Contributor

The data.table(..., stringsAsFactors = TRUE) idiosyncrasy popped up again in R/mod_mappingColumn.R. Updating that setting fixed the app for me. The only other issue I noticed is the Shift Table {Tplyr} + {kable} module was producing errors until I manually loaded Tplyr and kableExtra.

The Participant Selector module will save me sooo much time so thank you all for that! Please let me know how else I can help.

dirs<-paste(.libPaths(),'safetycharts','config', sep="/")
for(lib in .libPaths()){
dirs<-paste(lib,'safetyCharts','config', sep="/")
if(file.exists(dirs)) break
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would it be useful to add an error msg after the loop if dirs are not found stop('no chart config found, please install safetyCharts first'). If we force safetyCharts as a dependency pkg then it won't be an issue.

samussiah and others added 3 commits March 23, 2021 15:38
Add chart ordering via YAML and fix DataTables issue in Participant Selector.
add warning when no charts are found. remove stray renderers. update …
@jwildfire
Copy link
Contributor Author

Looks great! I added in @xni7 suggested warning (also created the related issues #517 for later) and cleaned up one failed test. Merging to dev now.

@jwildfire jwildfire merged commit 87fb7ab into dev Apr 13, 2021
@jwildfire jwildfire deleted the fix-441 branch April 22, 2021 14:43
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.

Support users with multiple lib paths

3 participants