feat: Add graph_join() function to R interface#2411
Conversation
- Add join() function and %j% operator in R/operators.R - Add comprehensive tests for join() in test-operators.R - Update documentation with examples - Export join and %j% in NAMESPACE Co-authored-by: krlmlr <1741643+krlmlr@users.noreply.github.com>
Co-authored-by: krlmlr <1741643+krlmlr@users.noreply.github.com>
join() function to R interface
|
@copilot: The merge updated instructions, adapt. |
Co-authored-by: krlmlr <1741643+krlmlr@users.noreply.github.com>
Updated the documentation to use proper math notation ( |
krlmlr
left a comment
krlmlr
left a comment
There was a problem hiding this comment.
Looks good, unsure about authorship, haven't checked.
|
@copilot: Merge |
Align with project convention to use named arguments for all _impl function calls. Co-authored-by: krlmlr <1741643+krlmlr@users.noreply.github.com>
Fix R CMD check WARNING by documenting the x and y parameters used by the %j% operator. Co-authored-by: krlmlr <1741643+krlmlr@users.noreply.github.com>
Yes, ran Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
| #' ## A star and a ring | ||
| #' g1 <- make_star(10, mode = "undirected") | ||
| #' g2 <- make_ring(5) | ||
| #' print_all(g1 %j% g2) |
There was a problem hiding this comment.
do we need print_all?
There was a problem hiding this comment.
Copilot should be taught to prefer plotting instead of printing graphs. I wanted to comment on some of these issues today but my wrist is not doing well, can't type. I fell on the ice and hurt it a few days ago. I'll get back to you later. Dictation doesn't work that well yet on macOS.
There was a problem hiding this comment.
Sorry to hear that, hope your wrist is better.
We could review the pkgdown rendering of all examples and decide on a course of actions. I'm not sure I'm on board with plotting by default just yet.
|
I'd call this |
|
Why is this operation not called No strong opinion re naming. |
|
The usual name for this operation is join. I don't know why it's called join, but the term "cross" evokes a product for me, so I would not have called it that. |
|
19 other packages export a function by that name: https://github.com/search?q=org%3Acran+%2F%5B%28%5Djoin%5B%29%5D%2F+path%3ANAMESPACE&type=code |
|
@copilot: Merge |
Rename the function from join() to graph_join() to avoid name conflicts with other packages and make the purpose clearer. The %j% operator is retained for convenience. Co-authored-by: krlmlr <1741643+krlmlr@users.noreply.github.com>
Merged Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
join() function to R interfacegraph_join() function to R interface
|
@copilot: Merge |
Add lifecycle experimental badge and @concept tag per AGENTS.md requirements for new functions. Co-authored-by: krlmlr <1741643+krlmlr@users.noreply.github.com>
Merged
All tests pass (7335 total), devtools::check() passes with 0 errors, 0 warnings, 0 notes. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
| #' | ||
| #' Both graphs need to have the same directedness, i.e. either both directed or | ||
| #' both undirected. If both graphs are directed, then for each pair of vertices | ||
| #' v, u in graphs G1, G2 we add edges (v, u) and (u, v) to maintain completeness. |
| ensure_igraph(g1) | ||
| ensure_igraph(g2) | ||
|
|
||
| on.exit(.Call(R_igraph_finalizer)) |
Co-authored-by: krlmlr <1741643+krlmlr@users.noreply.github.com>
Summary: Add
igraph_join()to RSuccessfully added the
graph_join()function to the R interface for igraph, exposing theigraph_join()C function that was added in version 0.10.11.Implementation Completed
graph_join()R function similar todisjoint_union()inR/operators.R%j%for convenience (similar to%du%for disjoint_union)tests/testthat/test-operators.Rgraph_join()to avoid name conflicts with other packagesWhat is
graph_join()?The
graph_join()function creates the join of two disjoint graphs by:For graphs with \eqn{|V_1|}, \eqn{|V_2|} vertices and \eqn{|E_1|}, \eqn{|E_2|} edges:
Usage Examples
Naming
The function is named
graph_join()(rather than justjoin()) to avoid conflicts with the 19 other CRAN packages that export a function namedjoin(), and to make it clear this is a graph-specific operation.Testing
All tests pass:
%j%AGENTS.md Compliance
✓ Examples included
✓ Tests included
✓ Proper documentation with arguments and return values
✓ Concept tag for pkgdown reference index
✓ Experimental badge via lifecycle
✓ Named arguments in _impl calls
✓ Math notation using \eqn{}
✓ Final check with devtools::check()
Fixes Add
igraph_join()to R #1345Original prompt
igraph_join()to R #1345💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.