Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Jan 24, 2026

Integrates callback functionality into motifs() as an optional parameter instead of exporting a separate function. Aligns API with cluster_leading_eigen() pattern.

Changes

  • motifs() function: Added optional callback parameter

    • Returns motif counts when callback = NULL (existing behavior)
    • Calls motifs_randesu_callback_closure_impl() and returns NULL invisibly when callback provided
  • motifs_randesu_callback() function: Removed entirely (not exported)

  • Tests: Updated to use motifs(callback = ...) syntax

  • Documentation:

    • Added callback parameter docs and example showing early search termination
    • Changed "3-6" to "3 to 6"
    • Removed redundant 1-based indexing clarification

Example

g <- sample_pa(100)

# Count all motifs (existing behavior)
motifs(g, 3)

# Stop search after finding 5 motifs (new)
count <- 0
motifs(g, 3, callback = function(vids, isoclass) {
  count <<- count + 1
  count < 5
})
  • 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>Refine callback implementation</issue_title>
<issue_description>Address post-merge feedback in #2465 . Specifically, integrate the new _impl into the top-level motifs() function instead of exporting a new function.</issue_description>

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


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI and others added 3 commits January 24, 2026 17:22
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] Refine callback implementation for motifs function refactor: integrate callback into motifs() function Jan 24, 2026
Copilot AI requested a review from krlmlr January 24, 2026 17:32
R/motifs.R Outdated
#'
#' This function searches for motifs in a graph and calls a user-provided
#' callback function for each motif found.
#' @description
Copy link
Contributor

Choose a reason for hiding this comment

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

Drop the function entirely, not needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in 1130037. The motifs_randesu_callback() function has been removed entirely. The callback functionality is now only available through the motifs(callback = ...) parameter.

Co-authored-by: krlmlr <1741643+krlmlr@users.noreply.github.com>
Copilot AI requested a review from krlmlr January 24, 2026 18:16
@krlmlr krlmlr marked this pull request as ready for review January 24, 2026 18:31
Copilot AI review requested due to automatic review settings January 24, 2026 18:31
@krlmlr krlmlr enabled auto-merge (squash) January 24, 2026 18:31
Copy link

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 the motif search callback functionality by integrating it as an optional parameter into the existing motifs() function, following the pattern established by cluster_leading_eigen(). This simplifies the API by removing the need for a separate exported function.

Changes:

  • Integrated callback functionality into motifs() as an optional parameter, maintaining backward compatibility
  • Removed motifs_randesu_callback() function (no longer exported)
  • Updated all tests and documentation to use the new motifs(callback = ...) syntax

Reviewed changes

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

Show a summary per file
File Description
R/motifs.R Added callback parameter to motifs() function; removed standalone motifs_randesu_callback() function; fixed error message formatting
tests/testthat/test-motifs.R Updated all callback tests to use new motifs(callback = ...) API
tests/testthat/_snaps/motifs.md Updated snapshot test header to reflect new API naming
man/motifs.Rd Added callback parameter documentation and example; improved text clarity ("3 to 6" instead of "3-6")
man/motifs_randesu_callback.Rd Removed documentation file for deleted function
man/sample_motifs.Rd Removed reference to deleted motifs_randesu_callback() function
man/graph.motifs.Rd Updated text clarity ("3 to 6" instead of "3-6")
man/dyad_census.Rd Removed reference to deleted motifs_randesu_callback() function
man/count_motifs.Rd Removed reference to deleted motifs_randesu_callback() function
NAMESPACE Removed export of motifs_randesu_callback()
.gitignore Minor formatting fix (added leading slash for consistency)

@krlmlr krlmlr merged commit 99d1b8a into main Jan 24, 2026
1 check passed
@krlmlr krlmlr deleted the copilot/refine-callback-implementation branch January 24, 2026 18:53
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.

Refine callback implementation

2 participants