search

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

Do not assume that `igraph::vcount` is of type `integer` #34

Closed szhorvat closed 11 months ago

szhorvat commented 11 months ago

Hi @mbojan,

There's code in the vignette of this package that assumes that igraph::vcount returns an integer and igraph::ecount returns a numeric.

https://github.com/mbojan/rgraph6/blob/master/vignettes/rgraph6.Rmd#L165

Generally, we do not guarantee the type, and in an upcoming version vcount will also return a numeric. The reason is to be able to support graphs with more vertices than .Machine$integer.max (similar to long vectors).

Could you please fix the vignette so that it doesn't assume this?

There's no new igraph version to test yet, but work is ongoing here.

mbojan commented 11 months ago

Yep will do. Thanks for the heads-up @szhorvat

szhorvat commented 11 months ago

Would it be better to just use sapply so as not to assume any specific type?

mbojan commented 11 months ago

I prefer vapply() to sapply() because of exactly that -- catch some unexpected type, which is unlikely in this context. It is not a "problem" because integer is numeric, that is is.numeric(integer(1)) is TRUE.

szhorvat commented 11 months ago

In the upcoming release, 1.6.0, the return type of vcount() is still integer. Originally, it was going to change only once we update to 0.10. Since this type change caused failures with several packages, should we just change it to numeric right now, in 1.6.0, @krlmlr? See the discussion and commit above.

mbojan commented 11 months ago

Network with 2 * 10^9 nodes or edges is a big one... :)

Perhaps @szhorvat it'd be better to continue in the igraph repository?