-
Notifications
You must be signed in to change notification settings - Fork 0
tests(visualizeNetworksWithHTML): Add unit tests for generateCytoscapeConfig #55
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -281,8 +281,9 @@ createEdgeElements <- function(edges) { | |
| #' used to render a network visualization. It's decoupled from any specific | ||
| #' UI framework. | ||
| #' | ||
| #' @param node_elements List of node elements created by createNodeElements() | ||
| #' @param edge_elements List of edge elements created by createEdgeElements() | ||
| #' @param nodes List of nodes from getSubnetworkFromIndra | ||
| #' @param edges List of edges from getSubnetworkFromIndra | ||
| #' @param display_label_type column of nodes table for displaying node names | ||
| #' @param container_id ID of the HTML container element (default: 'network-cy') | ||
| #' @param event_handlers Optional list of event handler configurations | ||
| #' @param layout_options Optional list of layout configuration options | ||
|
|
@@ -293,11 +294,16 @@ createEdgeElements <- function(edges) { | |
| #' - layout: Layout configuration | ||
| #' - container_id: Container element ID | ||
| #' - js_code: Complete JavaScript code (for backward compatibility) | ||
| generateCytoscapeConfig <- function(node_elements, edge_elements, | ||
| generateCytoscapeConfig <- function(nodes, edges, | ||
| display_label_type = "id", | ||
| container_id = "network-cy", | ||
| event_handlers = NULL, | ||
| layout_options = NULL) { | ||
|
|
||
| # Create elements | ||
| node_elements <- createNodeElements(nodes, display_label_type) | ||
| edge_elements <- createEdgeElements(edges) | ||
|
|
||
|
Comment on lines
+303
to
+306
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Validate inputs before element creation + fix downstream NA label handling Two actionable items:
Minimal validation immediately before element creation: - # Create elements
+ # Validate required columns before element creation
+ req_node_cols <- c("id")
+ miss_node <- setdiff(req_node_cols, names(nodes))
+ if (length(miss_node) > 0) {
+ stop("nodes is missing required column(s): ", paste(miss_node, collapse = ", "))
+ }
+ req_edge_cols <- c("source", "target", "interaction")
+ miss_edge <- setdiff(req_edge_cols, names(edges))
+ if (length(miss_edge) > 0) {
+ stop("edges is missing required column(s): ", paste(miss_edge, collapse = ", "))
+ }
+
+ # Create elements
node_elements <- createNodeElements(nodes, display_label_type)
edge_elements <- createEdgeElements(edges)Outside this hunk (in createNodeElements), replace the apply() block with a NA-safe, vectorised version like: createNodeElements <- function(nodes, displayLabelType = "id") {
# colors as before ...
if ("logFC" %in% names(nodes)) node_colors <- mapLogFCToColor(nodes$logFC) else node_colors <- rep("#D3D3D3", nrow(nodes))
label_column <- if (displayLabelType == "hgncName" && "hgncName" %in% names(nodes)) "hgncName" else "id"
df <- nodes
df$color <- node_colors
labels <- if (label_column == "hgncName") ifelse(!is.na(df$hgncName) & nzchar(df$hgncName), df$hgncName, df$id) else df$id
# optionally escape single quotes in labels/ids to avoid breaking JS strings
esc <- function(x) gsub("'", "\\\\'", x, fixed = TRUE)
mapply(function(id, label, color) {
paste0("{ data: { id: '", esc(id), "', label: '", esc(label), "', color: '", color, "' } }")
}, df$id, labels, df$color, USE.NAMES = FALSE)
}🤖 Prompt for AI Agents |
||
| # Default layout options | ||
| default_layout <- list( | ||
| name = "dagre", | ||
|
|
@@ -517,8 +523,6 @@ convertLayoutToJS <- function(layout_list) { | |
| #' @examples | ||
| #' \dontrun{ | ||
| #' # Assuming you have nodes and edges data | ||
| #' node_elements <- createNodeElements(nodes) | ||
| #' edge_elements <- createEdgeElements(edges) | ||
| #' config <- generateCytoscapeConfig(node_elements, edge_elements) | ||
| #' | ||
| #' # Export to HTML | ||
|
|
@@ -894,12 +898,8 @@ exportNetworkToHTML <- function(nodes, edges, | |
| displayLabelType = "id", | ||
| ...) { | ||
|
|
||
| # Create elements | ||
| node_elements <- createNodeElements(nodes, displayLabelType) | ||
| edge_elements <- createEdgeElements(edges) | ||
|
|
||
| # Generate configuration | ||
| config <- generateCytoscapeConfig(node_elements, edge_elements) | ||
| config <- generateCytoscapeConfig(nodes, edges, display_label_type = displayLabelType) | ||
|
|
||
| # Export to HTML | ||
| exportCytoscapeToHTML(config, filename, ...) | ||
|
|
@@ -917,13 +917,9 @@ exportNetworkToHTML <- function(nodes, edges, | |
| previewNetworkInBrowser <- function(nodes, edges, | ||
| displayLabelType = "id", | ||
| ...) { | ||
|
|
||
| # Create elements | ||
| node_elements <- createNodeElements(nodes, displayLabelType) | ||
| edge_elements <- createEdgeElements(edges) | ||
|
|
||
|
|
||
| # Generate configuration | ||
| config <- generateCytoscapeConfig(node_elements, edge_elements) | ||
| config <- generateCytoscapeConfig(nodes, edges, display_label_type = displayLabelType) | ||
|
|
||
| # Create temporary filename | ||
| temp_file <- tempfile(fileext = ".html") | ||
|
|
||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
API change to accept raw nodes/edges looks good; verify all call sites are updated
Great simplification. Please ensure no callers still pass node_elements/edge_elements.
Run this to find any outdated usages:
🏁 Script executed:
Length of output: 4102
Update documentation examples to use
nodes, edgesinstead ofnode_elements, edge_elementsThe code itself no longer has any live call sites passing the old parameter names, but the examples and manual pages still reference them. Please update the following:
-#' config <- generateCytoscapeConfig(node_elements, edge_elements)
+#' config <- generateCytoscapeConfig(nodes, edges)
#'
#' # Export to HTML
#' }
Once those are adjusted, all references to the old API will be removed.
📝 Committable suggestion
🤖 Prompt for AI Agents