Intended behaviour of `[<-.igraph` #1662

schochastics · 2025-01-19T15:42:17Z

Trying to add some more tests for #1661, I noticed some probably not intended behaviours of the current implementation

library(igraph)
#> 
#> Attaching package: 'igraph'
#> The following objects are masked from 'package:stats':
#> 
#>     decompose, spectrum
#> The following object is masked from 'package:base':
#> 
#>     union
packageVersion("igraph")
#> [1] '2.1.2'
g1 <- make_empty_graph(n = 10, directed = FALSE)
g1[1:5,] <- 1
g1[]
#> 10 x 10 sparse Matrix of class "dgCMatrix"
#>                          
#>  [1,] 1 2 2 2 2 1 1 1 1 1
#>  [2,] 2 1 2 2 2 1 1 1 1 1
#>  [3,] 2 2 1 2 2 1 1 1 1 1
#>  [4,] 2 2 2 1 2 1 1 1 1 1
#>  [5,] 2 2 2 2 1 1 1 1 1 1
#>  [6,] 1 1 1 1 1 . . . . .
#>  [7,] 1 1 1 1 1 . . . . .
#>  [8,] 1 1 1 1 1 . . . . .
#>  [9,] 1 1 1 1 1 . . . . .
#> [10,] 1 1 1 1 1 . . . . .

g2 <- make_empty_graph(n = 10, directed = FALSE)
g2[1:5,1:5] <- 1
g2[]
#> 10 x 10 sparse Matrix of class "dgCMatrix"
#>                          
#>  [1,] 1 2 2 2 2 . . . . .
#>  [2,] 2 1 2 2 2 . . . . .
#>  [3,] 2 2 1 2 2 . . . . .
#>  [4,] 2 2 2 1 2 . . . . .
#>  [5,] 2 2 2 2 1 . . . . .
#>  [6,] . . . . . . . . . .
#>  [7,] . . . . . . . . . .
#>  [8,] . . . . . . . . . .
#>  [9,] . . . . . . . . . .
#> [10,] . . . . . . . . . .

^{Created on 2025-01-19 with reprex v2.1.1}

In my opinion, this should rather look like this

A1 <- matrix(0,10,10)
A1[1:5,] <- 1
diag(A1) <- 0
A1
#>       [,1] [,2] [,3] [,4] [,5] [,6] [,7] [,8] [,9] [,10]
#>  [1,]    0    1    1    1    1    1    1    1    1     1
#>  [2,]    1    0    1    1    1    1    1    1    1     1
#>  [3,]    1    1    0    1    1    1    1    1    1     1
#>  [4,]    1    1    1    0    1    1    1    1    1     1
#>  [5,]    1    1    1    1    0    1    1    1    1     1
#>  [6,]    0    0    0    0    0    0    0    0    0     0
#>  [7,]    0    0    0    0    0    0    0    0    0     0
#>  [8,]    0    0    0    0    0    0    0    0    0     0
#>  [9,]    0    0    0    0    0    0    0    0    0     0
#> [10,]    0    0    0    0    0    0    0    0    0     0

A2 <- matrix(0,10,10)
A2[1:5,1:5] <- 1
diag(A2) <- 0
A2
#>       [,1] [,2] [,3] [,4] [,5] [,6] [,7] [,8] [,9] [,10]
#>  [1,]    0    1    1    1    1    0    0    0    0     0
#>  [2,]    1    0    1    1    1    0    0    0    0     0
#>  [3,]    1    1    0    1    1    0    0    0    0     0
#>  [4,]    1    1    1    0    1    0    0    0    0     0
#>  [5,]    1    1    1    1    0    0    0    0    0     0
#>  [6,]    0    0    0    0    0    0    0    0    0     0
#>  [7,]    0    0    0    0    0    0    0    0    0     0
#>  [8,]    0    0    0    0    0    0    0    0    0     0
#>  [9,]    0    0    0    0    0    0    0    0    0     0
#> [10,]    0    0    0    0    0    0    0    0    0     0

^{Created on 2025-01-19 with reprex v2.1.1}

thoughts @szhorvat @krlmlr ?

krlmlr · 2025-01-19T15:55:25Z

Looks legit:

options(conflicts.policy = list(warn = FALSE))
library(igraph)
packageVersion("igraph")
#> [1] '2.1.3'

g1 <- make_empty_graph(n = 6, directed = TRUE)
g1[1:3, ] <- 0.5
g1
#> IGRAPH bc20a3c D--- 6 18 -- 
#> + edges from bc20a3c:
#>  [1] 1->1 1->2 1->3 1->4 1->5 1->6 2->1 2->2 2->3 2->4 2->5 2->6 3->1 3->2 3->3
#> [16] 3->4 3->5 3->6
g1[]
#> 6 x 6 sparse Matrix of class "dgCMatrix"
#>                 
#> [1,] 1 1 1 1 1 1
#> [2,] 1 1 1 1 1 1
#> [3,] 1 1 1 1 1 1
#> [4,] . . . . . .
#> [5,] . . . . . .
#> [6,] . . . . . .

g2 <- make_empty_graph(n = 6, directed = FALSE)
E(g2)$weight <- 1
g2[1:3, 1:3] <- -2
g2
#> IGRAPH 9d11d1c U-W- 6 9 -- 
#> + attr: weight (e/n)
#> + edges from 9d11d1c:
#> [1] 1--1 1--2 1--3 1--2 2--2 2--3 1--3 2--3 3--3
g2[]
#> 6 x 6 sparse Matrix of class "dgCMatrix"
#>                    
#> [1,] -2 -4 -4 . . .
#> [2,] -4 -2 -4 . . .
#> [3,] -4 -4 -2 . . .
#> [4,]  .  .  . . . .
#> [5,]  .  .  . . . .
#> [6,]  .  .  . . . .

^{Created on 2025-01-19 with reprex v2.1.1}

I wonder if we want to add a loops = TRUE argument so that we can use g[..., loops = FALSE] to leave the diagonal alone.

krlmlr · 2025-01-19T15:56:22Z

Perhaps we shouldn't change the weight twice in the weighted case (second example).

schochastics · 2025-01-19T17:01:50Z

the loop argument is a good idea and it should default to FALSE

schochastics · 2025-01-19T20:58:34Z

PR #1661 now implements the following logic (loops = FALSE in all cases)

pkgload::load_all("~/git/R_packages/rigraph/")
#> ℹ Loading igraph
g1 <- make_empty_graph(n = 10, directed = FALSE)
g1[1:5, ] <- 1
g1[]
#> 10 x 10 sparse Matrix of class "dgCMatrix"
#>                          
#>  [1,] . 1 1 1 1 1 1 1 1 1
#>  [2,] 1 . 1 1 1 1 1 1 1 1
#>  [3,] 1 1 . 1 1 1 1 1 1 1
#>  [4,] 1 1 1 . 1 1 1 1 1 1
#>  [5,] 1 1 1 1 . 1 1 1 1 1
#>  [6,] 1 1 1 1 1 . . . . .
#>  [7,] 1 1 1 1 1 . . . . .
#>  [8,] 1 1 1 1 1 . . . . .
#>  [9,] 1 1 1 1 1 . . . . .
#> [10,] 1 1 1 1 1 . . . . .

#same as above
g1 <- make_empty_graph(n = 10, directed = FALSE)
g1[, 1:5] <- 1
g1[]
#> 10 x 10 sparse Matrix of class "dgCMatrix"
#>                          
#>  [1,] . 1 1 1 1 1 1 1 1 1
#>  [2,] 1 . 1 1 1 1 1 1 1 1
#>  [3,] 1 1 . 1 1 1 1 1 1 1
#>  [4,] 1 1 1 . 1 1 1 1 1 1
#>  [5,] 1 1 1 1 . 1 1 1 1 1
#>  [6,] 1 1 1 1 1 . . . . .
#>  [7,] 1 1 1 1 1 . . . . .
#>  [8,] 1 1 1 1 1 . . . . .
#>  [9,] 1 1 1 1 1 . . . . .
#> [10,] 1 1 1 1 1 . . . . .

#outgoing edges
g1 <- make_empty_graph(n = 10, directed = TRUE)
g1[1:5, ] <- 1
g1[]
#> 10 x 10 sparse Matrix of class "dgCMatrix"
#>                          
#>  [1,] . 1 1 1 1 1 1 1 1 1
#>  [2,] 1 . 1 1 1 1 1 1 1 1
#>  [3,] 1 1 . 1 1 1 1 1 1 1
#>  [4,] 1 1 1 . 1 1 1 1 1 1
#>  [5,] 1 1 1 1 . 1 1 1 1 1
#>  [6,] . . . . . . . . . .
#>  [7,] . . . . . . . . . .
#>  [8,] . . . . . . . . . .
#>  [9,] . . . . . . . . . .
#> [10,] . . . . . . . . . .

#incoming edges
g1 <- make_empty_graph(n = 10, directed = TRUE)
g1[ ,1:5] <- 1
g1[]
#> 10 x 10 sparse Matrix of class "dgCMatrix"
#>                          
#>  [1,] . 1 1 1 1 . . . . .
#>  [2,] 1 . 1 1 1 . . . . .
#>  [3,] 1 1 . 1 1 . . . . .
#>  [4,] 1 1 1 . 1 . . . . .
#>  [5,] 1 1 1 1 . . . . . .
#>  [6,] 1 1 1 1 1 . . . . .
#>  [7,] 1 1 1 1 1 . . . . .
#>  [8,] 1 1 1 1 1 . . . . .
#>  [9,] 1 1 1 1 1 . . . . .
#> [10,] 1 1 1 1 1 . . . . .

g1 <- make_empty_graph(n = 10, directed = TRUE)
g1[1 ,2] <- 1
g1[2, 1] <- 1
g1[]
#> 10 x 10 sparse Matrix of class "dgCMatrix"
#>                          
#>  [1,] . 1 . . . . . . . .
#>  [2,] 1 . . . . . . . . .
#>  [3,] . . . . . . . . . .
#>  [4,] . . . . . . . . . .
#>  [5,] . . . . . . . . . .
#>  [6,] . . . . . . . . . .
#>  [7,] . . . . . . . . . .
#>  [8,] . . . . . . . . . .
#>  [9,] . . . . . . . . . .
#> [10,] . . . . . . . . . .


g2 <- make_empty_graph(n = 10, directed = FALSE)
g2[6:10, 1:5] <- 1
g2[]
#> 10 x 10 sparse Matrix of class "dgCMatrix"
#>                          
#>  [1,] . . . . . 1 1 1 1 1
#>  [2,] . . . . . 1 1 1 1 1
#>  [3,] . . . . . 1 1 1 1 1
#>  [4,] . . . . . 1 1 1 1 1
#>  [5,] . . . . . 1 1 1 1 1
#>  [6,] 1 1 1 1 1 . . . . .
#>  [7,] 1 1 1 1 1 . . . . .
#>  [8,] 1 1 1 1 1 . . . . .
#>  [9,] 1 1 1 1 1 . . . . .
#> [10,] 1 1 1 1 1 . . . . .

g2 <- make_empty_graph(n = 10, directed = TRUE)
g2[6:10, 1:5] <- 1
g2[]
#> 10 x 10 sparse Matrix of class "dgCMatrix"
#>                          
#>  [1,] . . . . . . . . . .
#>  [2,] . . . . . . . . . .
#>  [3,] . . . . . . . . . .
#>  [4,] . . . . . . . . . .
#>  [5,] . . . . . . . . . .
#>  [6,] 1 1 1 1 1 . . . . .
#>  [7,] 1 1 1 1 1 . . . . .
#>  [8,] 1 1 1 1 1 . . . . .
#>  [9,] 1 1 1 1 1 . . . . .
#> [10,] 1 1 1 1 1 . . . . .


g2 <- make_empty_graph(n = 10, directed = TRUE)
g2[1:5, 6:10] <- 1
g2[]
#> 10 x 10 sparse Matrix of class "dgCMatrix"
#>                          
#>  [1,] . . . . . 1 1 1 1 1
#>  [2,] . . . . . 1 1 1 1 1
#>  [3,] . . . . . 1 1 1 1 1
#>  [4,] . . . . . 1 1 1 1 1
#>  [5,] . . . . . 1 1 1 1 1
#>  [6,] . . . . . . . . . .
#>  [7,] . . . . . . . . . .
#>  [8,] . . . . . . . . . .
#>  [9,] . . . . . . . . . .
#> [10,] . . . . . . . . . .

g3 <- make_empty_graph(n = 10, directed = FALSE)
g3[1:5, 1:5, attr = "weight"] <- 2
g3[]
#> 10 x 10 sparse Matrix of class "dgCMatrix"
#>                          
#>  [1,] . 2 2 2 2 . . . . .
#>  [2,] 2 . 2 2 2 . . . . .
#>  [3,] 2 2 . 2 2 . . . . .
#>  [4,] 2 2 2 . 2 . . . . .
#>  [5,] 2 2 2 2 . . . . . .
#>  [6,] . . . . . . . . . .
#>  [7,] . . . . . . . . . .
#>  [8,] . . . . . . . . . .
#>  [9,] . . . . . . . . . .
#> [10,] . . . . . . . . . .

g4 <- make_empty_graph(n = 6, directed = FALSE)
E(g4)$weight <- 1
g4[1:3, 1:3] <- -2
g4[]
#> 6 x 6 sparse Matrix of class "dgCMatrix"
#>                    
#> [1,]  . -2 -2 . . .
#> [2,] -2  . -2 . . .
#> [3,] -2 -2  . . . .
#> [4,]  .  .  . . . .
#> [5,]  .  .  . . . .
#> [6,]  .  .  . . . .

g5 <- make_empty_graph(n = 10, directed = FALSE)
g5[1:5, ] <- 2
#> Warning: value greater than one but graph is not weighted and no attribute was
#> specified. Only unweighted edges are added.
g5[]
#> 10 x 10 sparse Matrix of class "dgCMatrix"
#>                          
#>  [1,] . 1 1 1 1 1 1 1 1 1
#>  [2,] 1 . 1 1 1 1 1 1 1 1
#>  [3,] 1 1 . 1 1 1 1 1 1 1
#>  [4,] 1 1 1 . 1 1 1 1 1 1
#>  [5,] 1 1 1 1 . 1 1 1 1 1
#>  [6,] 1 1 1 1 1 . . . . .
#>  [7,] 1 1 1 1 1 . . . . .
#>  [8,] 1 1 1 1 1 . . . . .
#>  [9,] 1 1 1 1 1 . . . . .
#> [10,] 1 1 1 1 1 . . . . .

^{Created on 2025-01-19 with reprex v2.1.1}

Note also the new warning if a weight attribute is specified for an unweighted graph and no attr is given.

Any special cases missing?

krlmlr · 2025-01-20T08:42:01Z

Right: In #1662 (comment), for the directed case, the 1--2 edge and others exist twice, missed that.

schochastics mentioned this issue Jan 19, 2025

fix!: Subset assignment of a graph avoids addition of double edges and ignores loops unless the new loops argument is set to TRUE #1661

Merged

schochastics closed this as completed in #1661 Jan 23, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Intended behaviour of `[<-.igraph` #1662

Intended behaviour of `[<-.igraph` #1662

schochastics commented Jan 19, 2025

krlmlr commented Jan 19, 2025

krlmlr commented Jan 19, 2025

schochastics commented Jan 19, 2025

schochastics commented Jan 19, 2025 •

edited

Loading

krlmlr commented Jan 20, 2025 •

edited

Loading

Intended behaviour of [<-.igraph #1662

Intended behaviour of [<-.igraph #1662

Comments

schochastics commented Jan 19, 2025

krlmlr commented Jan 19, 2025

krlmlr commented Jan 19, 2025

schochastics commented Jan 19, 2025

schochastics commented Jan 19, 2025 • edited Loading

krlmlr commented Jan 20, 2025 • edited Loading

Intended behaviour of `[<-.igraph` #1662

Intended behaviour of `[<-.igraph` #1662

schochastics commented Jan 19, 2025 •

edited

Loading

krlmlr commented Jan 20, 2025 •

edited

Loading