refactor: replace .Call() with _impl functions and improve adjacency matrix implementation (#2546)

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: krlmlr <[email protected]>
Co-authored-by: Kirill Müller <[email protected]>
Co-authored-by: Kirill Müller <[email protected]>
Co-authored-by: Copilot <[email protected]>

Author: 3 people

R/aaa-auto.R

@@ -26,13 +26,14 @@ add_edges_impl <- function(
 ) {
   # Argument checks
   ensure_igraph(graph)
+  edges <- as_igraph_vs(graph, edges)
 
   on.exit(.Call(R_igraph_finalizer))
   # Function call
   res <- .Call(
     R_igraph_add_edges,
     graph,
-    edges
+    edges - 1
   )
 
   res
@@ -354,6 +355,7 @@ get_eids_impl <- function(
 ) {
   # Argument checks
   ensure_igraph(graph)
+  pairs <- as_igraph_vs(graph, pairs)
   directed <- as.logical(directed)
   error <- as.logical(error)
 
@@ -362,7 +364,7 @@ get_eids_impl <- function(
   res <- .Call(
     R_igraph_get_eids,
     graph,
-    pairs,
+    pairs - 1,
     directed,
     error
   )
@@ -7247,7 +7249,7 @@ edgelist_percolation_impl <- function(
   # Function call
   res <- .Call(
     R_igraph_edgelist_percolation,
-    edges
+    edges - 1
   )
 
   res
@@ -8769,6 +8771,7 @@ similarity_dice_pairs_impl <- function(
 ) {
   # Argument checks
   ensure_igraph(graph)
+  pairs <- as_igraph_vs(graph, pairs)
   mode <- switch_igraph_arg(
     mode,
     "out" = 1L,
@@ -8783,7 +8786,7 @@ similarity_dice_pairs_impl <- function(
   res <- .Call(
     R_igraph_similarity_dice_pairs,
     graph,
-    pairs,
+    pairs - 1,
     mode,
     loops
   )
@@ -8889,6 +8892,7 @@ similarity_jaccard_pairs_impl <- function(
 ) {
   # Argument checks
   ensure_igraph(graph)
+  pairs <- as_igraph_vs(graph, pairs)
   mode <- switch_igraph_arg(
     mode,
     "out" = 1L,
@@ -8903,7 +8907,7 @@ similarity_jaccard_pairs_impl <- function(
   res <- .Call(
     R_igraph_similarity_jaccard_pairs,
     graph,
-    pairs,
+    pairs - 1,
     mode,
     loops
   )

R/components.R

@@ -204,18 +204,15 @@ decompose <- function(
 ) {
   ensure_igraph(graph)
   mode <- igraph_match_arg(mode)
-  mode <- switch(mode, "weak" = 1L, "strong" = 2L)
 
   if (is.na(max.comps)) {
     max.comps <- -1
   }
-  on.exit(.Call(Rx_igraph_finalizer))
-  .Call(
-    Rx_igraph_decompose,
+  decompose_impl(
     graph,
-    as.numeric(mode),
-    as.numeric(max.comps),
-    as.numeric(min.vertices)
+    mode,
+    max.comps,
+    min.vertices
   )
 }
 

R/conversion.R

@@ -223,7 +223,6 @@ get.adjacency.dense <- function(
   graph,
   type = c("both", "upper", "lower"),
   attr = NULL,
-  weights = NULL,
   loops = c("once", "twice", "ignore"),
   names = TRUE
 ) {
@@ -243,20 +242,17 @@ get.adjacency.dense <- function(
     )
   }
   loops <- igraph_match_arg(loops)
-  loops <- switch(loops, "ignore" = 0L, "twice" = 1L, "once" = 2L)
-
-  if (!is.null(weights)) {
-    weights <- as.numeric(weights)
+  # Map "ignore" to "none" for get_adjacency_impl
+  if (loops == "ignore") {
+    loops <- "none"
   }
 
   if (is.null(attr)) {
-    on.exit(.Call(Rx_igraph_finalizer))
-    type <- switch(type, "upper" = 0, "lower" = 1, "both" = 2)
-    res <- .Call(
-      Rx_igraph_get_adjacency,
+    # FIXME: Use get_adjacency_impl() also for non-NULL attr
+    res <- get_adjacency_impl(
       graph,
-      as.numeric(type),
-      weights,
+      type,
+      weights = numeric(),
       loops
     )
   } else {
@@ -287,67 +283,33 @@ get.adjacency.sparse <- function(
 
   type <- igraph_match_arg(type)
 
-  vc <- vcount(graph)
-
-  el <- as_edgelist(graph, names = FALSE)
-  use.last.ij <- FALSE
-
-  if (!is.null(attr)) {
+  # Prepare weights parameter
+  if (is.null(attr)) {
+    weights <- numeric()
+  } else {
     attr <- as.character(attr)
     if (!attr %in% edge_attr_names(graph)) {
       cli::cli_abort("No such edge attribute", call = call)
     }
-    value <- edge_attr(graph, name = attr)
-    if (!is.numeric(value) && !is.logical(value)) {
+    weights <- edge_attr(graph, name = attr)
+    if (!is.numeric(weights) && !is.logical(weights)) {
       cli::cli_abort(
         "Matrices must be either numeric or logical, and the edge attribute is not",
         call = call
       )
     }
-  } else {
-    value <- rep(1, nrow(el))
   }
 
-  if (is_directed(graph)) {
-    res <- Matrix::sparseMatrix(
-      dims = c(vc, vc),
-      i = el[, 1],
-      j = el[, 2],
-      x = value,
-      use.last.ij = use.last.ij
-    )
-  } else {
-    if (type == "upper") {
-      ## upper
-      res <- Matrix::sparseMatrix(
-        dims = c(vc, vc),
-        i = pmin(el[, 1], el[, 2]),
-        j = pmax(el[, 1], el[, 2]),
-        x = value,
-        use.last.ij = use.last.ij
-      )
-    } else if (type == "lower") {
-      ## lower
-      res <- Matrix::sparseMatrix(
-        dims = c(vc, vc),
-        i = pmax(el[, 1], el[, 2]),
-        j = pmin(el[, 1], el[, 2]),
-        x = value,
-        use.last.ij = use.last.ij
-      )
-    } else if (type == "both") {
-      ## both
-      res <- Matrix::sparseMatrix(
-        dims = c(vc, vc),
-        i = pmin(el[, 1], el[, 2]),
-        j = pmax(el[, 1], el[, 2]),
-        x = value,
-        symmetric = TRUE,
-        use.last.ij = use.last.ij
-      )
-      res <- as(res, "generalMatrix")
-    }
-  }
+  # Use the library implementation
+  tmp <- get_adjacency_sparse_impl(
+    graph,
+    type,
+    weights,
+    loops = "once"
+  )
+
+  # Convert to proper Matrix object
+  res <- igraph.i.spMatrix(tmp)
 
   if (names && "name" %in% vertex_attr_names(graph)) {
     colnames(res) <- rownames(res) <- V(graph)$name
@@ -427,7 +389,6 @@ as_adjacency_matrix <- function(
       graph,
       type = type,
       attr = attr,
-      weights = NULL,
       names = names,
       loops = "once"
     )

R/flow.R

@@ -417,12 +417,11 @@ min_cut <- function(
     }
   } else {
     if (value.only) {
-      res <- .Call(
-        Rx_igraph_st_mincut_value,
-        graph,
-        as_igraph_vs(graph, source) - 1,
-        as_igraph_vs(graph, target) - 1,
-        capacity
+      res <- st_mincut_value_impl(
+        graph = graph,
+        source = source,
+        target = target,
+        capacity = capacity
       )
     } else {
       res <- st_mincut_impl(
@@ -526,12 +525,10 @@ vertex_connectivity <- function(
   if (is.null(source) && is.null(target)) {
     vertex_connectivity_impl(graph = graph, checks = checks)
   } else if (!is.null(source) && !is.null(target)) {
-    on.exit(.Call(Rx_igraph_finalizer))
-    .Call(
-      Rx_igraph_st_vertex_connectivity,
-      graph,
-      as_igraph_vs(graph, source) - 1,
-      as_igraph_vs(graph, target) - 1
+    st_vertex_connectivity_impl(
+      graph = graph,
+      source = source,
+      target = target
     )
   } else {
     cli::cli_abort(c(
@@ -631,12 +628,10 @@ edge_connectivity <- function(
   if (is.null(source) && is.null(target)) {
     edge_connectivity_impl(graph = graph, checks = checks)
   } else if (!is.null(source) && !is.null(target)) {
-    on.exit(.Call(Rx_igraph_finalizer))
-    .Call(
-      Rx_igraph_st_edge_connectivity,
-      graph,
-      as_igraph_vs(graph, source) - 1,
-      as_igraph_vs(graph, target) - 1
+    st_edge_connectivity_impl(
+      graph = graph,
+      source = source,
+      target = target
     )
   } else {
     cli::cli_abort(c(
@@ -653,12 +648,10 @@ edge_disjoint_paths <- function(graph, source = NULL, target = NULL) {
   if (is.null(source) || is.null(target)) {
     cli::cli_abort("Both source and target must be given")
   }
-  on.exit(.Call(Rx_igraph_finalizer))
-  .Call(
-    Rx_igraph_edge_disjoint_paths,
-    graph,
-    as_igraph_vs(graph, source) - 1,
-    as_igraph_vs(graph, target) - 1
+  edge_disjoint_paths_impl(
+    graph = graph,
+    source = source,
+    target = target
   )
 }
 
@@ -670,12 +663,10 @@ vertex_disjoint_paths <- function(graph, source = NULL, target = NULL) {
     cli::cli_abort("Both source and target must be given")
   }
 
-  on.exit(.Call(Rx_igraph_finalizer))
-  .Call(
-    Rx_igraph_vertex_disjoint_paths,
-    graph,
-    as_igraph_vs(graph, source) - 1,
-    as_igraph_vs(graph, target) - 1
+  vertex_disjoint_paths_impl(
+    graph = graph,
+    source = source,
+    target = target
   )
 }