Skip to content

chore: improve errors in iterators.R #2006

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

Open
maelle wants to merge 10 commits into
base: main
Choose a base branch
from
+311 −30
Open

chore: improve errors in iterators.R #2006

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
971a8c8
chore: improve errors in iterators.R
maelle Jul 22, 2025
2fd131c
Merge origin/main into stoop2
claude Jul 27, 2026
2be846a
tests thx Claude
maelle Aug 12, 2026
59eec13
expand attribute removal
maelle Aug 26, 2026
588cec2
better
maelle Aug 26, 2026
a4e0bc8
name name yay
maelle Aug 26, 2026
b367288
better errors
maelle Aug 26, 2026
a988499
better wording
maelle Aug 26, 2026
85883bc
ok
maelle Aug 26, 2026
f46e70f
more specific
maelle Aug 26, 2026
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
90 changes: 61 additions & 29 deletions R/iterators.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1132,7 +1132,7 @@ simple_es_index <- function(x, i, na_ok = FALSE) {
}
if (is.logical(ii) && (length(ii) != length(x) && length(ii) != 1)) {
cli::cli_abort(
"Error: Logical index length does not match the number of edges. Recycling is not allowed."
"Logical index length does not match the number of edges. Recycling is not allowed."
)
}

Expand Down Expand Up @@ -1197,11 +1197,19 @@ simple_es_index <- function(x, i, na_ok = FALSE) {
#' @name igraph-vs-attributes
#' @export
`[[<-.igraph.vs` <- function(x, i, value) {
if (!rlang::has_name(attributes(value), "attr_name")) {

@maelle maelle Aug 26, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I asked Claude to rename name and value to attr_name and attr_value in this context because I think the double "value" added to my confusion when I first looked at this code.

cli::cli_abort("Can't find {.val name} for vertex attribute.")
}
if (
!"name" %in% names(attributes(value)) ||
!"value" %in% names(attributes(value))
Comment thread
maelle marked this conversation as resolved.
!rlang::has_name(attributes(value), "attr_value") &&
!is_complete_iterator(value)
) {
cli::cli_abort("Invalid indexing.")
cli::cli_abort(
c(
"Can't find {.val value} for vertex attribute {.val {attr(value, 'attr_name')}}.",
i = "Removing an attribute is only supported for the whole vertex sequence, e.g. {.code V(g)${attr(value, 'attr_name')} <- NULL}, not a subset. Use {.fn delete_vertex_attr}."
)
)
}
if (is.null(get_vs_graph(x))) {
cli::cli_abort("Graph is unknown.", .internal = TRUE)
Expand All @@ -1219,11 +1227,19 @@ simple_es_index <- function(x, i, na_ok = FALSE) {
#' @name igraph-es-attributes
#' @export
`[[<-.igraph.es` <- function(x, i, value) {
if (!rlang::has_name(attributes(value), "attr_name")) {
cli::cli_abort("Can't find {.val name} for edge attribute.")
}
if (
!"name" %in% names(attributes(value)) ||
!"value" %in% names(attributes(value))
!rlang::has_name(attributes(value), "attr_value") &&
!is_complete_iterator(value)
) {
cli::cli_abort("Invalid indexing.")
cli::cli_abort(
c(
"Can't find {.val value} for edge attribute {.val {attr(value, 'attr_name')}}.",
i = "Removing an attribute is only supported for the whole edge sequence, e.g. {.code E(g)${attr(value, 'attr_name')} <- NULL}, not a subset. Use {.fn delete_edge_attr}."
)
)
}
if (is.null(get_es_graph(x))) {
cli::cli_abort("Graph is unknown.", .internal = TRUE)
Expand Down Expand Up @@ -1292,7 +1308,7 @@ simple_es_index <- function(x, i, na_ok = FALSE) {
`$.igraph.vs` <- function(x, name) {
graph <- get_vs_graph(x)
if (is.null(graph)) {
cli::cli_abort("Graph is unknown")
cli::cli_abort("Can't find graph.")
}
res <- vertex_attr(graph, name, x)
if (is_single_index(x)) {
Expand Down Expand Up @@ -1345,7 +1361,7 @@ simple_es_index <- function(x, i, na_ok = FALSE) {
`$.igraph.es` <- function(x, name) {
graph <- get_es_graph(x)
if (is.null(graph)) {
cli::cli_abort("Graph is unknown")
cli::cli_abort("Can't find graph.")
}
res <- edge_attr(graph, name, x)
if (is_single_index(x)) {
Expand All @@ -1363,10 +1379,10 @@ simple_es_index <- function(x, i, na_ok = FALSE) {
#' @export
`$<-.igraph.vs` <- function(x, name, value) {
if (is.null(get_vs_graph(x))) {
cli::cli_abort("Graph is unknown")
cli::cli_abort("Can't find graph.")
}
attr(x, "name") <- name
attr(x, "value") <- value
attr(x, "attr_name") <- name
attr(x, "attr_value") <- value
x
}

Expand All @@ -1378,28 +1394,36 @@ simple_es_index <- function(x, i, na_ok = FALSE) {
#' @family vertex and edge sequences
`$<-.igraph.es` <- function(x, name, value) {
if (is.null(get_es_graph(x))) {
cli::cli_abort("Graph is unknown")
cli::cli_abort("Can't find graph.")
}
attr(x, "name") <- name
attr(x, "value") <- value
attr(x, "attr_name") <- name
attr(x, "attr_value") <- value
x
}

#' @name igraph-vs-attributes
#' @export
`V<-` <- function(x, value) {
ensure_igraph(x)
if (
!"name" %in% names(attributes(value)) ||
!"value" %in% names(attributes(value))
) {
cli::cli_abort("invalid indexing")
if (!rlang::has_name(attributes(value), "attr_name")) {
cli::cli_abort("Can't find {.val name} for vertex attribute.")
}
if (!rlang::has_name(attributes(value), "attr_value")) {
if (is_complete_iterator(value)) {
return(delete_vertex_attr(x, attr(value, "attr_name")))
}
cli::cli_abort(
c(
"Can't find {.val value} for vertex attribute {.val {attr(value, 'attr_name')}}.",
i = "Removing an attribute is only supported for the whole vertex sequence, e.g. {.code V(g)${attr(value, 'attr_name')} <- NULL}, not a subset. Use {.fn delete_vertex_attr}."
)
)
}
i_set_vertex_attr(
x,
attr(value, "name"),
name = attr(value, "attr_name"),
index = value,
value = attr(value, "value"),
value = attr(value, "attr_value"),
check = FALSE
)
}
Expand All @@ -1413,17 +1437,25 @@ simple_es_index <- function(x, i, na_ok = FALSE) {
#' @export
`E<-` <- function(x, path = NULL, P = NULL, directed = NULL, value) {
ensure_igraph(x)
if (
!"name" %in% names(attributes(value)) ||
!"value" %in% names(attributes(value))
) {
cli::cli_abort("invalid indexing")
if (!rlang::has_name(attributes(value), "attr_name")) {
cli::cli_abort("Can't find {.val name} for edge attribute.")
}
if (!rlang::has_name(attributes(value), "attr_value")) {
if (is_complete_iterator(value)) {
return(delete_edge_attr(x, attr(value, "attr_name")))
}
cli::cli_abort(
c(
"Can't find {.val value} for edge attribute {.val {attr(value, 'attr_name')}}.",
i = "Removing an attribute is only supported for the whole edge sequence, e.g. {.code E(g)${attr(value, 'attr_name')} <- NULL}, not a subset. Use {.fn delete_edge_attr}."
)
)
}
i_set_edge_attr(
x,
attr(value, "name"),
name = attr(value, "attr_name"),
index = value,
value = attr(value, "value"),
value = attr(value, "attr_value"),
check = FALSE
)
}
Expand Down
126 changes: 125 additions & 1 deletion tests/testthat/_snaps/iterators.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,5 +106,129 @@
E(g)[c(TRUE, FALSE)]
Condition
Error in `FUN()`:
! Error: Logical index length does not match the number of edges. Recycling is not allowed.
! Logical index length does not match the number of edges. Recycling is not allowed.

# assigning `NULL` to a subset of vertices/edges errors instead of silently doing nothing

Code
V(g)[1:3]$color <- NULL
Condition
Error in `[<-`:
! Can't find "value" for vertex attribute "color".
i Removing an attribute is only supported for the whole vertex sequence, e.g. `V(g)$color <- NULL`, not a subset. Use `delete_vertex_attr()`.

---

Code
E(g)[1:3]$weight <- NULL
Condition
Error in `[<-`:
! Can't find "value" for edge attribute "weight".
i Removing an attribute is only supported for the whole edge sequence, e.g. `E(g)$weight <- NULL`, not a subset. Use `delete_edge_attr()`.

# assigning `NULL` for a non-existent attribute errors like `delete_vertex_attr()`/`delete_edge_attr()`

Code
V(g)$color <- NULL
Condition
Error in `delete_vertex_attr()`:
! No vertex attribute `color` found.

---

Code
E(g)$weight <- NULL
Condition
Error in `delete_edge_attr()`:
! No edge attribute `weight` found.

# direct misuse of `V<-`/`E<-`/`[<-`/`[[<-` errors well

Code
V(g) <- "blue"
Condition
Error in `V<-`:
! Can't find "name" for vertex attribute.

---

Code
E(g) <- "blue"
Condition
Error in `E<-`:
! Can't find "name" for edge attribute.

---

Code
V(g)[1] <- "blue"
Condition
Error in `[<-`:
! Can't find "name" for vertex attribute.

---

Code
E(g)[1] <- "blue"
Condition
Error in `[<-`:
! Can't find "name" for edge attribute.

---

Code
V(g)[[1]] <- "blue"
Condition
Error in `[[<-`:
! Can't find "name" for vertex attribute.

---

Code
E(g)[[1]] <- "blue"
Condition
Error in `[[<-`:
! Can't find "name" for edge attribute.

# querying or setting attributes errors when the graph is unknown

Code
vs$color
Condition
Error in `vs$color`:
! Can't find graph.

---

Code
vs$color <- "blue"
Condition
Error in `$<-`:
! Can't find graph.

---

Code
es$weight
Condition
Error in `es$weight`:
! Can't find graph.

---

Code
es$weight <- 0
Condition
Error in `$<-`:
! Can't find graph.

# `[<-.igraph.vs` reports an internal error when the graph is unknown

Code
`[<-.igraph.vs`(vs, 1, value = payload)
Condition
Error in `[<-.igraph.vs`:
! Graph is unknown.
i This is an internal error that was detected in the igraph package.
Please report it at <https://github.com/igraph/rigraph/issues> with a reprex (<https://tidyverse.org/help/>) and the full backtrace.

Loading
Loading