Bug fixes, remove drop_unused_query_nodes (breaking change)#73
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR removes the deprecated drop_unused_query_nodes parameter from the expand() family of functions (breaking change), updates all related documentation, tests, and namespace entries, and introduces fixes to preserve the active table context and prevent an infinite loop in the Neo4j engine expansion.
- Remove
drop_unused_query_nodesfrom R function signatures, man pages, vignettes, and tests - Add
active_tblcapture/restore around tidygraph operations to maintain the active context - Fix Neo4j conditional query grouping to avoid pagination infinite loops
Reviewed Changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| vignettes/monarchr.Rmd | Removed documentation block describing drop_unused_query_nodes |
| tests/testthat/test-expand.file_engine.R | Deleted tests referencing drop_unused_query_nodes |
| man/expand_n.Rd | Dropped parameter docs and updated @return tag |
| man/expand.Rd | Removed drop_unused_query_nodes from Rd entries |
| R/transitive_reduction.R | Capture and restore active table with active_tbl |
| R/transitive_closure.R | Short-circuit when no matching edges and restore active_tbl |
| R/kg_edge_weights.R | Capture and restore active_tbl around mutate calls |
| R/graph_semsim.R | Capture and restore active_tbl around edge mutation |
| R/graph_centrality.R | Capture and restore active_tbl around node mutation |
| R/fetch_nodes.neo4j_engine.R | Wrap conditional in parentheses to fix infinite loop |
| R/expand_neo4j_engine.R | Removed drop_unused_query_nodes logic |
| R/expand_n.R | Removed parameter and check_len calls for drop_unused_query_nodes |
| R/expand_file_engine.R | Removed drop_unused_query_nodes argument and branches |
| R/expand.tbl_kgx.R | Capture/restore active_tbl when dispatching to engines |
| R/expand.R | Dropped drop_unused_query_nodes from generic signature |
| NEWS.md | Notes breaking change and Neo4j bugfix |
| NAMESPACE | Imported active() from tidygraph |
| DESCRIPTION | Bumped package version to 2.0 |
Comments suppressed due to low confidence (2)
man/expand_n.Rd:14
- The
@returntag is split across two lines, which can break Rd rendering. Merge into one line, e.g.,#' @return Atbl_kgx()graph.
#' @return
R/fetch_nodes.neo4j_engine.R:66
- You’ve fixed the infinite-loop bug by grouping the WHERE clause in parentheses; add a unit test for the Neo4j pagination behavior to prevent future regressions.
query <- paste0("MATCH (n) WHERE (", generate_cypher_conditionals(...), ")")
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes issues #72 #70 #69 #58