insightsengineering / teal.modules.hermes

RNA-seq analysis modules to add to a teal application
https://insightsengineering.github.io/teal.modules.hermes/
Other
7 stars 1 forks source link

[Bug]: PCA module crashes when filters applied #319

Closed chlebowa closed 1 year ago

chlebowa commented 1 year ago

In RNA-seq app:

app crashes

image

danielinteractive commented 1 year ago

Thanks, does this happen locally as well?

chlebowa commented 1 year ago

I am unable to run the app locally with restore_and_run as helios Depends on R >= 4.3 and R 4.3 was not published for Ubuntu.

Running on my own installation (helios dependency lifted) I get this error:

Warning: Error in [[<-: 5 elements in value to replace 2 elements
  3: runApp
  2: print.shiny.appobj
  1: <Anonymous>
Warning: Error in [[<-: 5 elements in value to replace 2 elements
  3: runApp
  2: print.shiny.appobj
  1: <Anonymous>
Warning: Error in [[<-: 5 elements in value to replace 2 elements
  126: <Anonymous>
  125: stop
  124: pca_result
  123: <reactive>
  107: show_matrix_pca
  106: exprFunc
  105: widgetFunc
  104: ::
htmlwidgets
shinyRenderWidget
  103: func
   90: renderFunc
   89: renderFunc
   85: renderFunc
   84: output$teal-main_ui-root-pca_plot-module-table_pca
    3: runApp
    2: print.shiny.appobj
    1: <Anonymous>
Warning: Error in [[<-: 5 elements in value to replace 2 elements
  225: <Anonymous>
  224: stop
  223: pca_result
  222: <reactive>
  220: .func
  217: contextFunc
  216: env$runWith
  209: ctx$run
  208: self$.updateValue
  206: plot_r
  204: <reactive>
  202: .func
  199: contextFunc
  198: env$runWith
  191: ctx$run
  190: self$.updateValue
  188: plot_type
  187: <reactive>
  185: .func
  182: contextFunc
  181: env$runWith
  174: ctx$run
  173: self$.updateValue
  171: plot_reactive
  170: renderPlot
  168: func
  128: drawPlot
  114: <reactive:plotObj>
   98: drawReactive
   85: renderFunc
   84: output$teal-main_ui-root-pca_plot-module-plot_pca-plot_main
    3: runApp
    2: print.shiny.appobj
    1: <Anonymous>
danielinteractive commented 1 year ago

Wait this is not depending on helios, just hermes. Staged dependencies should help with installation

chlebowa commented 1 year ago

My mistake. The error I got here is of the same nature but comes from S4Arrays:

ERROR: this R is version 4.2.2, package 'S4Arrays' requires R >= 4.3.0
Error: install of package 'S4Arrays' failed [error code 1]
In addition: Warning message:
This project is configured to use Bioconductor 3.17, which is not compatible with R 4.2.2.
Use 'renv::init(bioconductor = "3.17")' to re-initialize this project with the appropriate Bioconductor release.

restore_and_run uses renv, I don't think sd will be of much help.

danielinteractive commented 1 year ago

Hm interesting. Does this mean we also depend now on R 4.3 with this package?

chlebowa commented 1 year ago

I'm actually not sure. The the renv in teal.gallery wants Bioconductor 3.17 which contains DelayedArray 0.26.6 which requires S4Arrays, which requires R 4.3. I have Bioconductor 3.16 withDelayedArray`0.24.0' and noS4Arrays` and the application runs.

danielinteractive commented 1 year ago

Ah ok. Could that be a mistake on the renv config then? Because teal.gallery would be supposed to support the current release on 4.2.2 or not?

chlebowa commented 1 year ago

@vedhav can you chip in?

vedhav commented 1 year ago

I see the problem. As you pointed out, I tried to restore in R 4.2.2, and it failed because of the S4Arrays installation.

However, I am able to install hermes from R 4.2.2 (without renv.lock) because it installs an older version from BioC which does not have the S4Arrays dependency (As it installs the SummarizedExperiment version 1.28.0). The problem arises because the renv.lock was generated with a newer version of the hermes package with an updated SummarizedExperiment version. So we could restore the renv.lock file with an older R version (4.2) with BioC 3.16 to fix this or just roll back the SummarizedExperiment version from GitHub.

But, I want to point out that maintaining an older version of renv.lock files are tricky. Currently, the plan is to make sure that we update the renv.lock file with the latest packages and make sure that the app works as expected.

danielinteractive commented 1 year ago

I would think that there is a general inconsistency here between the deployment environment R version (4.2.2) and the renv lock file (needs BioC 3.17 which needs R 4.3). Or not?

vedhav commented 1 year ago

The deployment environment R version is 4.3.1 (2023-06-16) which is in line with the renv.lock file.

danielinteractive commented 1 year ago

But on Ocean we don't have 4.3 yet or have we?

vedhav commented 1 year ago

Not sure about Ocean. But, the deployment is done by the CI image ghcr.io/insightsengineering/rstudio_4.3.1_bioc_3.17:latest here's the workflow that does that.

gogonzo commented 1 year ago

I confirm. I've followed steps @chlebowa described and app fails when range is changed. I'm on R 4.3.0

gogonzo commented 1 year ago

I think I've found the source of the problem. Looks like it is on us (coredev), I will continue tomorrow.

gogonzo commented 1 year ago

Closed by https://github.com/insightsengineering/teal.modules.hermes/pull/326