Skip to content

feat: accept two-column matrices/data frames in edge functions (#1827)#2700

Open
schochastics wants to merge 5 commits into
mainfrom
feat-1827-edge-matrix-args
Open

feat: accept two-column matrices/data frames in edge functions (#1827)#2700
schochastics wants to merge 5 commits into
mainfrom
feat-1827-edge-matrix-args

Conversation

@schochastics

Copy link
Copy Markdown
Contributor

Summary

Closes #1827. get_edge_ids() was changed in #1663 to accept the natural edge-list forms — an n×2 matrix or a two-column data frame — via the el_to_vec() helper. This PR applies the same treatment to the other functions that take edges as pairs of vertices, so the same edge list can be passed everywhere with no manual transposition (the core complaint in #550):

  • add_edges() / delete_edges()
  • edge() / edges() (via the + and - operators)
  • make_graph() (and make_directed_graph() / make_undirected_graph())
  • E(graph, P = )

For all of these, an n×2 matrix or two-column data frame is now the canonical edge-list form. 2×n and 2×2 matrices are deprecated (defunct), consistent with get_edge_ids().

⚠️ Behavior change for make_graph() (please read)

Unlike the other functions, make_graph() already accepted a matrix — but it flattened it column-major. The practical consequences:

  • A plain n×2 edge list (e.g. as_edgelist(g)) silently produced the wrong graph.
  • The transposed 2×n form (t(edgelist)) was the working idiom.

This PR makes the n×2 matrix the correct, canonical form and turns the 2×n form into a defunct error. This is a breaking change: code relying on make_graph(t(el)) must now pass make_graph(el) (n×2) directly. This is intentional and is exactly what #550 asked for, but it is more disruptive than for the other functions, which previously errored or mis-handled matrices rather than silently accepting a transposed one.

Implementation notes

  • el_to_vec() gains arg/fn parameters so error/deprecation messages name the calling function (e.g. "the edges argument of add_edges()"). Defaults preserve the existing get_edge_ids wording, so existing snapshots are unchanged.
  • Each entry point gates on is.data.frame(x) || inherits(x, "matrix") before normalizing, so vectors and igraph.vs/igraph.es sequences keep their existing behavior.
  • delete_edges() converts a matrix/df of vertex pairs to edge IDs via get_edge_ids(..., error = TRUE) (its edges arg is an edge selector, not a vertex sequence), preserving numeric-edge-id and "a|b" semantics for vectors.
  • Internal callers that relied on passing 2×n matrices are updated to pass explicit flat vectors (c(...)): graph_from_edgelist(), graph_from_data_frame(), the incidence/adjacency constructors, and [<- indexing. (graph_from_data_frame() was in fact already broken by the add_edges change and is fixed here.)

Tests

Added matrix/data-frame acceptance, vector/igraph.vs back-compat, 2×n/2×2 rejection, and as_edgelist() round-trip (#550) cases across test-interface.R, test-operators.R, test-make.R, and test-iterators.R. Updated test-conversion.R to use the now-natural n×2 form.

Full suite: 1553 test groups, 0 failures, 0 errors. No snapshot changes.

🤖 Generated with Claude Code

Extend the n×2 matrix / two-column data frame edge specification that
`get_edge_ids()` already supports (via `el_to_vec()`) to the other
functions that take edges as pairs of vertices:

- `add_edges()` / `delete_edges()`
- `edge()` / `edges()` (via the `+` and `-` operators)
- `make_graph()` (and `make_directed_graph()` / `make_undirected_graph()`)
- `E(graph, P = )`

For all of these an n×2 matrix or two-column data frame is now the natural
edge-list form; 2×n and 2×2 matrices are deprecated (defunct), consistent
with `get_edge_ids()`. This resolves the counterintuitive transposition
requirement from #550.

`make_graph()` previously accepted a matrix but flattened it column-major,
so a plain n×2 edge list produced the wrong graph and the transposed 2×n
form was the working idiom; that interpretation is now corrected.

Internal callers that passed 2×n matrices (graph_from_edgelist,
graph_from_data_frame, incidence/adjacency constructors, indexing) are
updated to pass explicit flat vectors. `el_to_vec()` gains `arg`/`fn`
parameters so error messages name the correct function.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

This is how benchmark results would change (along with a 95% confidence interval in relative change) if 713cdd8 is merged into main:

  • 🚀as_adjacency_matrix: 760ms -> 730ms [-4.46%, -3.39%]
  • 🚀as_biadjacency_matrix: 764ms -> 730ms [-4.76%, -3.94%]
  • ❗🐌as_data_frame_both: 1.49ms -> 1.51ms [+0.54%, +2.04%]
  • ✔️as_long_data_frame: 3.87ms -> 3.89ms [-0.03%, +1.23%]
  • ✔️es_attr_filter: 2.69ms -> 2.69ms [-0.95%, +0.75%]
  • 🚀graph_from_adjacency_matrix: 140ms -> 132ms [-8.62%, -2.9%]
  • ✔️graph_from_data_frame: 3.42ms -> 3.42ms [-0.78%, +0.88%]
  • ✔️vs_attr_filter: 1.53ms -> 1.53ms [-0.76%, +0.96%]
  • ✔️vs_by_name: 989µs -> 998µs [0%, +1.95%]
    Further explanation regarding interpretation and methodology can be found in the documentation.

schochastics and others added 2 commits June 18, 2026 07:49
The `distances()` roxygen example passed `t(el[, 1:2])` (a 2×n matrix) to
`add_edges()`, relying on the old column-major flattening. Since 2×n
matrices are now defunct, pass the n×2 matrix `el[, 1:2]` directly, which
produces the same graph. Fixes the R CMD check example failure.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

This is how benchmark results would change (along with a 95% confidence interval in relative change) if be3d283 is merged into main:

  • ✔️as_adjacency_matrix: 832ms -> 828ms [-2.92%, +1.95%]
  • ✔️as_biadjacency_matrix: 864ms -> 864ms [-1.2%, +1.22%]
  • ✔️as_data_frame_both: 1.7ms -> 1.7ms [-4.53%, +4.35%]
  • ✔️as_long_data_frame: 4.44ms -> 4.43ms [-6.13%, +5.93%]
  • ✔️es_attr_filter: 3.37ms -> 3.35ms [-7.57%, +6.39%]
  • ✔️graph_from_adjacency_matrix: 137ms -> 137ms [-2.52%, +3.63%]
  • ✔️graph_from_data_frame: 3.76ms -> 3.82ms [-4.45%, +7.94%]
  • ✔️vs_attr_filter: 1.89ms -> 1.87ms [-3.9%, +2.22%]
  • ✔️vs_by_name: 1.22ms -> 1.26ms [-1.54%, +7.03%]
    Further explanation regarding interpretation and methodology can be found in the documentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

adjust add_edges() and related functions to get_edge_ids()

1 participant