Skip to content

chore: Use autogen-impl functions for 3 motifs functions #2152

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
krlmlr merged 3 commits into from
Oct 19, 2025
Merged

chore: Use autogen-impl functions for 3 motifs functions #2152

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9629813
chore: use `motifs_randesu_no_impl()`
maelle Sep 22, 2025
cb71304
chore: use `motifs_randesu_estimate_impl()`
maelle Sep 22, 2025
e56d6c9
chore: use `motifs_randesu_impl()`
maelle Sep 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 10 additions & 30 deletions R/motifs.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,22 +151,14 @@ dyad.census <- function(graph) {
#' count_motifs(g, 3)
#' sample_motifs(g, 3)
motifs <- function(graph, size = 3, cut.prob = NULL) {
ensure_igraph(graph)

if (!is.null(cut.prob)) {
cut.prob <- as.numeric(cut.prob)
}

if (!is.null(cut.prob) && length(cut.prob) != size) {
cli::cli_abort("{arg cut.prob} must be the same length as {.arg size}")
}

on.exit(.Call(R_igraph_finalizer))
res <- .Call(
R_igraph_motifs_randesu,
res <- motifs_randesu_impl(
graph,
as.numeric(size),
cut.prob
size = size,
cut.prob = cut.prob
)
res[is.nan(res)] <- NA
res
Expand Down Expand Up @@ -200,20 +192,14 @@ motifs <- function(graph, size = 3, cut.prob = NULL) {
count_motifs <- function(graph, size = 3, cut.prob = NULL) {
ensure_igraph(graph)
Copy link
Copy Markdown
Contributor Author

@maelle maelle Sep 22, 2025

Choose a reason for hiding this comment

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

I am on the fence about removing it, because even if that is asserted in the autogenerated function, shouldn't this happen before anything else?

Copy link
Copy Markdown
Contributor

@schochastics schochastics Sep 22, 2025

Choose a reason for hiding this comment

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

Could you be more specific? You mean remove the ensure_igraph() call? Maybe for consistency with other functions keep it still?

Copy link
Copy Markdown
Contributor Author

@maelle maelle Sep 22, 2025

Choose a reason for hiding this comment

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

@schochastics actually it's not very consistent among functions taking graph as input, from the little I've seen. A new upkeep task! 🤪

Copy link
Copy Markdown
Contributor

@schochastics schochastics Sep 22, 2025

Choose a reason for hiding this comment

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

I am actually not surprised 🙈


if (!is.null(cut.prob)) {
cut.prob <- as.numeric(cut.prob)
}

if (!is.null(cut.prob) && length(cut.prob) != size) {
cli::cli_abort("{arg cut.prob} must be the same length as {.arg size}")
}

on.exit(.Call(R_igraph_finalizer))
.Call(
R_igraph_motifs_randesu_no,
motifs_randesu_no_impl(
graph,
as.numeric(size),
cut.prob
size = size,
cut.prob = cut.prob
)
}

Expand Down Expand Up @@ -258,10 +244,6 @@ sample_motifs <- function(
) {
ensure_igraph(graph)

if (!is.null(cut.prob)) {
cut.prob <- as.numeric(cut.prob)
}

if (!is.null(cut.prob) && length(cut.prob) != size) {
cli::cli_abort("{arg cut.prob} must be the same length as {.arg size}")
}
Expand All @@ -275,13 +257,11 @@ sample_motifs <- function(
sample.size <- 0
}

on.exit(.Call(R_igraph_finalizer))
.Call(
R_igraph_motifs_randesu_estimate,
motifs_randesu_estimate_impl(
graph,
as.numeric(size),
cut.prob,
as.numeric(sample.size),
size = size,
cut.prob = cut.prob,
sample.size = sample.size,
sample
)
}
Expand Down
Loading