Skip to content

refactor: replace .Call() expressions with _impl() function calls#2541

Merged
krlmlr merged 4 commits intomainfrom
copilot/replace-call-expressions-with-impl-functions
Jan 24, 2026
Merged

refactor: replace .Call() expressions with _impl() function calls#2541
krlmlr merged 4 commits intomainfrom
copilot/replace-call-expressions-with-impl-functions

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Jan 24, 2026

Replaced 8 .Call() expressions with their corresponding _impl() wrappers, aligning with the codebase convention to avoid .Call() outside *_impl() in aaa-auto.R.

Changes

Files modified:

  • R/adjacency.R: adjacency, weighted_adjacency
  • R/make.R: create_bipartite (2 occurrences)
  • R/operators.R: complementer, difference (2 occurrences)
  • R/conversion.R: get_edgelist

Key refactoring: Updated graph.adjacency.dense() to pass string mode/loops parameters directly to _impl() functions instead of pre-converting to integers, since _impl() functions handle this internally.

Rationale for non-replaced calls

Remaining .Call() expressions (~40) cannot be replaced due to:

  • Signature mismatches: _impl() functions missing parameters (subset, which, edgemaps, circle)
  • No _impl() exists: Functions like mybracket2, get_attr_mode, set_verbose
  • Intentional exceptions: e.g., vcount in warn_version() to avoid recursion

Prepared with GitHub Copilot assistance.

  • By submitting this pull request, I assign the copyright of my contribution to The igraph development team.
Original prompt

This section details on the original issue you should resolve

<issue_title>Replace 30 or so .Call() expressions with calls to _impl functions</issue_title>
<issue_description>Only where possible. Do not introduce new _impl functions.</issue_description>

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI and others added 3 commits January 24, 2026 12:54
Co-authored-by: krlmlr <1741643+krlmlr@users.noreply.github.com>
Co-authored-by: krlmlr <1741643+krlmlr@users.noreply.github.com>
Co-authored-by: krlmlr <1741643+krlmlr@users.noreply.github.com>
Copilot AI changed the title [WIP] Replace .Call() expressions with calls to _impl functions refactor: replace .Call() expressions with _impl() function calls Jan 24, 2026
Copilot AI requested a review from krlmlr January 24, 2026 13:00
@krlmlr krlmlr marked this pull request as ready for review January 24, 2026 17:20
Copilot AI review requested due to automatic review settings January 24, 2026 17:20
@krlmlr krlmlr enabled auto-merge (squash) January 24, 2026 17:20
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors R code to replace direct .Call() expressions with their corresponding _impl() wrapper functions, aligning with the codebase convention that .Call() should only be used within auto-generated *_impl() functions in aaa-auto.R.

Changes:

  • Replaced 8 direct .Call() expressions across 4 files with calls to their corresponding _impl() wrapper functions
  • Updated parameter passing in adjacency.R to use string values ("none", "twice", "once") instead of integers, which are then converted by the _impl() functions internally
  • Removed redundant on.exit(.Call(R*_igraph_finalizer)) calls since these are now handled within the _impl() functions

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
R/operators.R Replaced .Call() in difference.igraph() (2 occurrences) and complementer() with difference_impl() and complementer_impl() calls
R/make.R Replaced .Call() in graph.bipartite() and make_bipartite_graph() with create_bipartite_impl() calls
R/conversion.R Replaced .Call() in as_edgelist() with get_edgelist_impl() call
R/adjacency.R Replaced .Call() in graph.adjacency.dense() with adjacency_impl() and weighted_adjacency_impl() calls; updated parameter conversion to pass strings instead of integers

@krlmlr krlmlr disabled auto-merge January 24, 2026 17:44
@krlmlr krlmlr merged commit 8c4a582 into main Jan 24, 2026
11 checks passed
@krlmlr krlmlr deleted the copilot/replace-call-expressions-with-impl-functions branch January 24, 2026 17:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Replace 30 or so .Call() expressions with calls to _impl functions

3 participants