fix: `biconnected_components()` returns invalid result fields
The auto-generated biconnected_components_impl() was creating duplicate empty fields with dot notation (tree.edges, component.edges, articulation.points) alongside the correct underscore fields from the C layer.
Root Cause
The impl function received underscore field names from C but incorrectly accessed them using dot notation:
res$tree.edges <- lapply(res$tree.edges, ...) # res$tree.edges doesn't exist yet
This created new empty dot fields while leaving the original underscore fields unchanged, resulting in:
> names(res)
[1] "no" "tree_edges" "component_edges" "components" "articulation_points"
[6] "tree.edges" "component.edges" "articulation.points" # Empty duplicates
Changes
-
R/aaa-auto.R: Fixed field access to use underscore notation (
res$tree_edges,res$component_edges,res$articulation_points) - R/components.R: Removed redundant workaround that was recreating underscore fields and deleting empty dot fields
- tests/testthat/_snaps/aaa-auto.md: Updated snapshot to reflect correct output format
Result
> res <- biconnected_components(make_graph(c(1,2, 2,3, 3,1, 1,4), directed = FALSE))
> names(res)
[1] "no" "tree_edges" "component_edges" "components" "articulation_points"
All fields now contain properly typed igraph.vs/igraph.es objects.
Original prompt
This section details on the original issue you should resolve
<issue_title>biconnected_components() returns invalid result fields</issue_title>
<issue_description>biconnected_components() returns a result list with named components tree_edges, component_edges, articulation_points. These list components have valid values, However, the result also has components tree.edges, component.edges and articulation.points. These are empty.
Note that biconnected_components() is fully auto-generated, but somewhere during auto-generation the _ is changed to a ., but only in some places, not everywhere.
> res<-biconnected_components(make_graph(c(1,2, 2,3, 3,1, 1,4), directed = F))
> res
$no
[1] 2
$tree_edges
$tree_edges[[1]]
[1] 2 1
$tree_edges[[2]]
[1] 4
$component_edges
$component_edges[[1]]
[1] 3 2 1
$component_edges[[2]]
[1] 4
$components
$components[[1]]
+ 3/4 vertices, from c488bbb:
[1] 3 2 1
$components[[2]]
+ 2/4 vertices, from c488bbb:
[1] 4 1
$articulation_points
[1] 1
$articulation.points
+ 0/4 vertices, from c488bbb:
Also notice the function definition:
biconnected_components_impl <- function(graph) {
# Argument checks
ensure_igraph(graph)
on.exit( .Call(R_igraph_finalizer) )
# Function call
res <- .Call(R_igraph_biconnected_components, graph)
if (igraph_opt("return.vs.es")) {
res$tree.edges <- lapply(res$tree.edges, unsafe_create_es, graph = graph, es = E(graph))
}
if (igraph_opt("return.vs.es")) {
res$component.edges <- lapply(res$component.edges, unsafe_create_es, graph = graph, es = E(graph))
}
if (igraph_opt("return.vs.es")) {
res$components <- lapply(res$components, unsafe_create_vs, graph = graph, verts = V(graph))
}
if (igraph_opt("return.vs.es")) {
res$articulation.points <- create_vs(graph, res$articulation.points)
}
res
}
</issue_description>
Comments on the Issue (you are @copilot in this section)
This specific function used underscore names in 1.2.11, res$articulation_points
The autogenerated code in 1.3.0 uses a buggy mixture of underscore and dot, and ends up with the mess I showed above. Furthermore, the outputs are no longer a proper vertex/edges sequences.
As Tamás said in chat, we need to decide whether we go with dots or underscores for output parameter names. Input parameters and attributes already use dots, so that's a natural choice, but that would involve a breaking change for biconnected_components().</comment_new>
<comment_new>
igraph_biconnected_components:
PARAM_NAMES:
articulation_points: articulation_points
tree_edges: tree_edges
component_edges: component_edges
but this doesn't disable the _ -> . replacement. If the parameters are renamed to anything but the original name, then the replacement is disabled. It would be good to fix this in Stimulus, to allow for special cases when we DON'T want the .-renaming.</comment_new>
<comment_new>
_ -> . replacement is specific to the R code generator and it was there to enforce R's then-preferred dotted notation without having to spell out the alternative names one by one for each argument. Also, this came before we've had PARAM_NAMES. I wouldn't try to fix the treatment of PARAM_NAMES vs the replacement, I'd rather prefer first to decide once and for all which of the two formats (dotted or underscored) is preferred in the R interface. If it is the dotted notation, then everything is fine and we don't need any special treatment in PARAM_NAMES. If it is the underscored notation, we need to remove the replacement code and that's all.</comment_new>
<comment_new>I'd rather prefer first to decide once and for all which of the two formats (dotted or underscored) is preferred in the R interface. If it is the dotted notation, then everything is fine and we don't need any special treatment in
PARAM_NAMES. If it is the underscored notation, we need to remove the replacement code and that's all.
@krlmlr, the ball's in your court on this one.</comment_new>
<comment_new>
Just breaking the functionality seems to require a...
- Fixes igraph/rigraph#1203
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.