Skip to content

Conversation

maelle
Copy link
Contributor

@maelle maelle commented Sep 22, 2025

Note that these high-level functions are tested.

@maelle maelle changed the title Use autogen-impl functions for two motifs functions Use autogen-impl functions for 3 motifs functions Sep 22, 2025
#' count_motifs(g, 3)
#' sample_motifs(g, 3)
count_motifs <- function(graph, size = 3, cut.prob = NULL) {
ensure_igraph(graph)
Copy link
Contributor Author

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
Contributor

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
Contributor Author

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
Contributor

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 🙈

@maelle maelle marked this pull request as ready for review September 22, 2025 08:26
@maelle maelle requested a review from schochastics September 30, 2025 08:35
@maelle
Copy link
Contributor Author

maelle commented Sep 30, 2025

Other comments @schochastics?

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.

2 participants