Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pca worflow integrated #146

Merged
merged 23 commits into from
Dec 11, 2023
Merged

Pca worflow integrated #146

merged 23 commits into from
Dec 11, 2023

Conversation

PaulJonasJost
Copy link
Collaborator

No description provided.

Copy link
Collaborator

@LeaSeep LeaSeep left a comment

Choose a reason for hiding this comment

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

Testing now on server

@@ -16,7 +15,7 @@ source("R/enrichment_analysis/translation.R", local = T)
source("R/enrichment_analysis/server.R", local = T)
source("R/heatmap/server.R",local = T)
source("R/pca/server.R", local = T)
source("R/volcano_plot/server.R", local = T)
source("R/pca/util.R", local = T)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we want to name it utils.Rto be consistent with all utils files?

adjustMethod
adjustMethod,
comp_type,
ref,
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe renaming to `ctrl' - to be more clear

input$sample_annotation_types_cmp_GSEA,
input$Groups2Compare_ref_GSEA,
input$Groups2Compare_treat_GSEA,
input$ValueToAttach
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
input$ValueToAttach
input$Value_to_attach

@@ -238,6 +237,9 @@ heatmap_server <- function(id, data, params, updates){
input$PreProcessing_Procedure
)

### atm raw data plotted
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this a todo? Shouldn't the processed data be always used further?

print(input$row_anno_options_heatmap)
additionalInput_row_anno <- ifelse(any(input$row_selection_options == "rowAnno_based"),"yip",NA)
if(!is.na(additionalInput_row_anno)){
additionalInput_row_anno <- input$anno_options_heatmap
print(additionalInput_row_anno)
}
additionalInput_row_anno_factor <- input$row_anno_options_heatmap
#} # check if this is working if yes delete lines
#} # check if this is working if yes delete lines # @leaseep, is this checked
Copy link
Collaborator

Choose a reason for hiding this comment

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

No, and I honestly do not remember what the issues was here.. Do you know? (As you just worked through this code base?)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

no, could not find anything specific.

program/shinyApp/R/significance_analysis/ui.R Outdated Show resolved Hide resolved
program/shinyApp/R/significance_analysis/util.R Outdated Show resolved Hide resolved
Comment on lines +253 to +254
# workaround, as somehow the input values dont show up unless we change it in the shiny
# TODO: fix this (@Lea?)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I may misunderstand, but one reason could be that we only get the value of an reactive if it is actually called (or triggered due to change of that respective variable)
So to get the input values through 'toPlotVolcano' - reacive you need to wrap this in an 'observerEvent' or similiar

program/shinyApp/R/significance_analysis/util.R Outdated Show resolved Hide resolved
@@ -39,6 +39,7 @@ library(kableExtra)
library(readxl)
library(ggvenn)
library(ComplexUpset)
library(gridExtra) # TODO: needs to be added to renv
Copy link
Collaborator

Choose a reason for hiding this comment

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

should be available as a requirement for multiple other packages

@LeaSeep
Copy link
Collaborator

LeaSeep commented Oct 25, 2023

Image somehow still broken but less important for now

@PaulJonasJost PaulJonasJost merged commit a044abb into develop Dec 11, 2023
0 of 2 checks passed
@PaulJonasJost PaulJonasJost deleted the pca_worflow_integrated branch December 11, 2023 13:04
@PaulJonasJost PaulJonasJost restored the pca_worflow_integrated branch December 16, 2023 19:51
@PaulJonasJost PaulJonasJost deleted the pca_worflow_integrated branch December 16, 2023 19:52
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.

2 participants