node_diversity() test and igraph update

szhorvat commented 2 years ago

Hello @thomasp85,

I am writing on behalf of the igraph project. igraph 1.3.0, to be released soon, makes some changes to the diversity() function, which causes the tidygraph test suite to fail. See here and below:

https://github.com/thomasp85/tidygraph/blob/master/tests/testthat/test-node_measures.R#L13

There are two problems:

"Diversity", as calculated by this function, only makes sense for undirected graphs. The test uses a directed graph. Starting with 1.3.0, igraph will not accept a directed input. This is the cause of the failure.
"Diversity" is meaningful for vertices which have more than one connection. The test here uses a tree, which has vertices with degree one. Currently, igraph returns both NaN and -Inf for some vertices. This is inconsistent, and furthermore I believe that the more reasonable result is zero, for which I just opened a PR for igraph. I recommend testing the diversity() function with a graph in which all vertices have a degree greater than one, and all weights are strictly positive. Since the tests do not check the specific values that are returned, using the current graph would allow the test to pass, provided that it is converted to undirected first.

thomasp85 commented 2 years ago

Thanks for the heads-up. This will get fixed today.

I'm also seeing a test error with automorphisms that is more difficult to comprehend https://cran.r-project.org/web/checks/check_results_tidygraph.html

Does that test error make sense to you?

szhorvat commented 2 years ago

This happens because these functions got a new argument, colors, which is now the second argument instead of sh. In principle, the fix should be to use something like automorphisms(graph, sh='fs') instead of simply automorphisms(graph, 'fs'). However, the sh argument seems broken and will only accept 'fm', it seems. Ping @ntamas and I'll open an issue for this soon.

szhorvat commented 2 years ago

See https://github.com/igraph/rigraph/issues/518

szhorvat commented 2 years ago

Just as a heads up, igraph 1.3.1, fixing the issue I referred to, should be submitted sometime this week.

thomasp85 / tidygraph

node_diversity() test and igraph update #151