mbojan / rgraph6

Encoding graphs in graph6, sparse6, and digraph6 formats
https://mbojan.github.io/rgraph6/
GNU General Public License v3.0
12 stars 3 forks source link

Fix border cases #20

Closed mbojan closed 2 years ago

mbojan commented 4 years ago

These do not work at this moment for graph6. Need to check for dgraph6 and sparse6 too.

> # Graph of size 0
> (g6 <- rawToChar(as.raw(fN(0))))
[1] "?"
> as_adjacency(g6)

 Error in rep(what, l - length(x)) : invalid 'times' argument

> 
> # Graph of size 1
> (g6 <- rawToChar(as.raw(fN(1))))
[1] "@"
> as_adjacency(g6)

 Error in rep(what, l - length(x)) : invalid 'times' argument

Edit: work is done on i20-border-cases branch.

schochastics commented 2 years ago

Moving discussion here. My fix was only for the *_from_*() functions. From your tests in the i20-border-case branch is see you also want to cover those cases in the as_*6() functions. I will try to do this (i.e. get the failed tests to not fail ;-)). Are you fine with hardcoding the n<2 cases for graph6 and digraph6? I did a lot of testing and the empty graphs with 0 and 1 nodes are the only cases I could find that have an issue. For sparse6, all empty graphs with n nodes fail so that needs a more general solution I guess (scratch that, I see you already did some work there)

schochastics commented 2 years ago

After fixing the boundary cases, some of the tests fail because the output types are different e.g. line 95-102 in test-i20-border-cases.R fails because igraph_from_sparse6() returns a list and not an igraph object

Would it be ok to change such tests to

expect_true(igraph::identical_graphs(
    igraph_from_sparse6(string)[[1]],
    g0u
  ))
mbojan commented 2 years ago

After fixing the boundary cases, some of the tests fail because the output types are different e.g. line 95-102 in test-i20-border-cases.R fails because igraph_from_sparse6() returns a list and not an igraph object

Would it be ok to change such tests to

expect_true(igraph::identical_graphs(
    igraph_from_sparse6(string)[[1]],
    g0u
  ))

Yes. I think its best for these functions to always return a list even if dealing with a single string.(much like strsplit()).