Skip to content

perf: speed up .apply_modifiers() in make.R#2600

Draft
Copilot wants to merge 2 commits intomainfrom
copilot/remove-todo-speed-up-make-r
Draft

perf: speed up .apply_modifiers() in make.R#2600
Copilot wants to merge 2 commits intomainfrom
copilot/remove-todo-speed-up-make-r

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 27, 2026

Removes the ## TODO: speed this up comments in .apply_modifiers() by implementing the actual speedups.

Changes

  • without_attr: Replace three O(N) loops (get names → delete one-by-one) with three direct bulk assignments to setNames(list(), character()) — O(1) regardless of attribute count.
  • with_vertex_ / with_edge_: Replace per-attribute set_vertex_attr/set_edge_attr calls (each doing a full C-level get+modify+set cycle) with a single get → in-memory loop with scalar recycling and length validation → single set. Reduces from N get+set pairs to 1 get + 1 set.
  • with_graph_: Replace per-attribute loop with modifyList() + graph.attributes<- for a single get+set cycle.
  • Add snapshot tests for wrong-length attribute value errors in with_vertex_() and with_edge_().
  • By submitting this pull request, I assign the copyright of my contribution to The igraph development team.

🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

Copilot AI changed the title [WIP] Remove TODO comment from make.R perf: speed up .apply_modifiers() in make.R Mar 27, 2026
Copilot AI requested a review from schochastics March 27, 2026 19:04
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.

can we remove the "## TODO: speed this up" in make.R?

2 participants