Network Visualization core logic by pnavada · Pull Request #99 · Vitek-Lab/MSstatsShiny

pnavada · 2024-11-18T01:05:11Z

PR Type

enhancement

Description

Implemented core logic for network visualization using Cytoscape.js in the server module.
Developed a UI module to support network visualization, including data upload and settings.
Integrated the network server module into the main server function.
Added a new tab for network interpretation in the UI.

Changes walkthrough 📝

Relevant files

Enhancement

module-visualize-network-server.R `Implement network visualization server logic with Cytoscape.js` R/module-visualize-network-server.R Added a server module to handle network visualization. Implemented reactive expressions for data processing and network rendering. Integrated Cytoscape.js for network visualization. Added error handling for CSV data upload.	+114/-0
module-visualize-network-ui.R `Develop UI for network visualization with Cytoscape.js` R/module-visualize-network-ui.R Created a UI module for network visualization. Integrated Cytoscape.js and related libraries. Added UI components for data upload and network settings.	+69/-0
server.R `Integrate network server module into main server` R/server.R Integrated the network server module into the main server function.	+2/-0
ui.R `Add network interpretation tab to UI` R/ui.R Added a new tab for network interpretation in the UI.	+1/-0

💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

github-actions · 2024-11-18T01:06:22Z

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 No relevant tests
🔒 Security concerns JavaScript Injection Risk: The implementation directly uses user inputs in JavaScript code generation, which could lead to JavaScript injection vulnerabilities. This is particularly concerning in lines where node and edge data are dynamically generated based on user input. Consider sanitizing these inputs or employing other methods to ensure the security of the generated code.
⚡ Recommended focus areas for review Error Handling The error handling in the CSV reading function could be improved by providing more specific error messages or handling different types of errors differently. JavaScript Injection Risk Directly using user input in JavaScript code generation could lead to JavaScript injection vulnerabilities. Consider sanitizing or validating the inputs before use. Performance Concern The loop for edge elements creation might be inefficient for large datasets. Consider optimizing this section or using vectorized operations if possible.

github-actions · 2024-11-18T01:06:55Z

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Score
Possible bug	Add error handling for data processing functions to improve robustness Consider adding error handling for the `annotateProteinInfoFromIndra` and `getSubnetworkFromIndra` functions to manage potential failures or exceptions in data processing. R/module-visualize-network-server.R [33-34] -annotated_df <- annotateProteinInfoFromIndra(df(), proteinIdType()) -subnetwork <- getSubnetworkFromIndra(annotated_df, pValue()) +annotated_df <- tryCatch({ + annotateProteinInfoFromIndra(df(), proteinIdType()) +}, error = function(e) { + showNotification(paste("Error in annotation:", e$message), type = "error") + return(NULL) +}) +subnetwork <- tryCatch({ + getSubnetworkFromIndra(annotated_df, pValue()) +}, error = function(e) { + showNotification(paste("Error in subnetwork extraction:", e$message), type = "error") + return(NULL) +}) Suggestion importance[1-10]: 8 Why: Adding error handling for the functions `annotateProteinInfoFromIndra` and `getSubnetworkFromIndra` is crucial to manage potential failures, improving the robustness and user experience of the application.	8
Possible bug	Ensure numeric inputs are valid to prevent runtime errors Validate the numeric conversion of `input$pValue` and `input$logFC` to handle non-numeric inputs gracefully. R/module-visualize-network-server.R [20-28] pValue <- reactive({ req(input$pValue) - as.numeric(input$pValue) + if (!is.na(as.numeric(input$pValue))) { + as.numeric(input$pValue) + } else { + showNotification("Invalid pValue input", type = "error") + return(NULL) + } }) logFC <- reactive({ req(input$logFC) - as.numeric(input$logFC) + if (!is.na(as.numeric(input$logFC))) { + as.numeric(input$logFC) + } else { + showNotification("Invalid logFC input", type = "error") + return(NULL) + } }) Suggestion importance[1-10]: 7 Why: Validating the numeric conversion of inputs `pValue` and `logFC` is essential to prevent runtime errors from non-numeric inputs, thus enhancing the application's stability and user experience.	7
Performance	Implement caching to reduce redundant data processing and enhance responsiveness Implement caching for the `df`, `proteinIdType`, `pValue`, and `logFC` reactive expressions to improve performance by avoiding redundant computations. R/module-visualize-network-server.R [4-28] df <- reactive({ req(input$dataUpload) - tryCatch({ - read.csv(input$dataUpload$datapath) - }, error = function(e) { - ... + isolate({ + if (!exists("cached_df")) { + cached_df <<- tryCatch({ + read.csv(input$dataUpload$datapath) + }, error = function(e) { + ... + }) + } + cached_df }) }) proteinIdType <- reactive({ req(input$proteinIdType) - input$proteinIdType + isolate({ + if (!exists("cached_proteinIdType")) { + cached_proteinIdType <<- input$proteinIdType + } + cached_proteinIdType + }) }) ... Suggestion importance[1-10]: 6 Why: Implementing caching for reactive expressions can significantly enhance performance by reducing redundant computations, especially beneficial in scenarios with frequent data updates or large datasets.	6
Performance	Use more efficient methods for generating JavaScript code to enhance performance Optimize the JavaScript code generation by using more efficient string concatenation techniques or pre-compiled templates. R/module-visualize-network-server.R [54-60] -js_code <- paste0(" - cytoscape.use(cytoscapeDagre); - var cy = cytoscape({ - container: document.getElementById('network-cy'), - elements: [", paste(elements, collapse = ", "), "], - ... - }); - ") +js_template <- "cytoscape.use(cytoscapeDagre); + var cy = cytoscape({ + container: document.getElementById('network-cy'), + elements: [%s], + ... + });" +js_code <- sprintf(js_template, paste(elements, collapse = ", ")) Suggestion importance[1-10]: 5 Why: Optimizing JavaScript code generation using templates can improve performance, although the impact might be moderate depending on the overall application size and complexity.	5

…ep 5

tonywu1999 · 2024-12-04T21:59:20Z

@@ -0,0 +1,176 @@
+networkServer <- function(input, output, session, parent_session, significant) {


nit: Could you rename this to visualizeNetworkServer? I think networkServer may confuse people thinking it has something to do with computer networks.

tonywu1999 · 2024-12-04T22:01:43Z

+        if (is.null(input$dataUpload) && !is.null(significant())) {
+            df <- significant()
+            if (!is.null(df) && "Protein" %in% names(df)) {
+                df$Protein <- as.character(significant()$Protein)


Code would be easier to follow if you do df$Protein <- as.character(df$Protein)

tonywu1999 · 2024-12-04T22:06:22Z

  return(
    list(
      input = input,
+      significant = SignificantProteins,


I think this is where we had talked about using the proteins being already filtered before coming to step 5 network viz.

I think adding a significant parameter to the returned object may not be necessary since you can use the dataComparison$ComparisonResult table as the list of proteins with their p-values

tonywu1999 · 2024-12-04T22:07:25Z

      data_comp = data_comparison()
      significant = data_comp$ComparisonResult[
        which(data_comp$ComparisonResult$adj.pvalue < input$signif), ]
-      print(significant)


Any reason for removing this print statement here?

added it back

tonywu1999 · 2024-12-04T22:09:45Z

+    renderNetwork <- reactive({
+        # Annotate protein info and filter the subnetwork based on p-value
+        annotated_df <- tryCatch({
+                annotateProteinInfoFromIndra(df(), proteinIdType())


Did you add a dependency on MSstatsBioNet for this package? i.e. using the @importFrom tag and adding the dependency in the DESCRIPTION file.

tonywu1999 · 2024-12-04T22:14:18Z

+            interaction <- edge_parts[3]
+
+            # Get edges table
+            edges_table <- renderNetwork()$edges_table


Will this call INDRA again? If so, could we use the original edges table that we had already.

No, it wont call INDRA again. It happens only when the inputs change

tonywu1999 · 2024-12-04T22:23:44Z

+    # Observe the event when an edge is clicked
+    observeEvent(input$edgeClicked, {
+        edge_id <- input$edgeClicked$id
+        edge_parts <- strsplit(edge_id, "-")[[1]]


rather than a string split, I believe we already have source, target, and interaction data stored in the edge as well. Would it be simpler to pass and retrieve that information instead of the ID itself?

tonywu1999 · 2024-12-04T22:25:50Z

                      selected = "Uploaddata")
  })

+  callModule(networkServer, "network", session, significant)


Instead of passing significant, could you instead pass data_comparison$ComparisonResult instead here?

* adding network visualization core logic * linking step 5 to the pipeline and adding nodes table to output in step 5 * highlighting the selected edge from the network in edges table * error handling for indra method calls * swapping the positions of edges and nodes table in ui * PR suggestions * PR suggestions

adding network visualization core logic

20409af

pnavada changed the title ~~Adding network visualization core logic~~ Network Visualization core logic Nov 18, 2024

github-actions Bot added the Review effort [1-5]: 4 label Nov 18, 2024

pnavada added 3 commits November 24, 2024 17:35

linking step 5 to the pipeline and adding nodes table to output in st…

e796b57

…ep 5

highlighting the selected edge from the network in edges table

437dd39

error handling for indra method calls

00152e4

tonywu1999 requested review from devonjkohler and tonywu1999 November 25, 2024 22:06

swapping the positions of edges and nodes table in ui

6cd7b57

tonywu1999 changed the base branch from devel to devel-bionet December 4, 2024 16:29

tonywu1999 reviewed Dec 4, 2024

View reviewed changes

pnavada added 2 commits January 6, 2025 09:14

PR suggestions

4dce2e7

PR suggestions

79909ae

tonywu1999 merged commit e7fd143 into devel-bionet Jan 9, 2025

tonywu1999 deleted the indra-integration branch January 9, 2025 15:58

This was referenced Jan 9, 2025

[Pruthvi] Integrate MSstatsBioNet with MSstatsShiny Vitek-Lab/MSstatsBioNet#23

Closed

[Pruthvi] Add new function cytoscapeJS Vitek-Lab/MSstatsBioNet#24

Closed

		@@ -0,0 +1,176 @@
		networkServer <- function(input, output, session, parent_session, significant) {

Conversation

pnavada commented Nov 18, 2024 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Type

Description

Changes walkthrough 📝

Uh oh!

github-actions Bot commented Nov 18, 2024

PR Reviewer Guide 🔍

Uh oh!

github-actions Bot commented Nov 18, 2024

PR Code Suggestions ✨

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tonywu1999 Dec 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tonywu1999 Dec 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tonywu1999 Dec 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

pnavada commented Nov 18, 2024 •

edited by github-actions Bot

Loading

tonywu1999 Dec 4, 2024 •

edited

Loading

tonywu1999 Dec 4, 2024 •

edited

Loading

tonywu1999 Dec 4, 2024 •

edited

Loading