igraph / rigraph

igraph R package
https://r.igraph.org
557 stars 202 forks source link

test: add tests for as_adjacency_matrix() in test-conversion #1519

Closed maelle closed 2 months ago

maelle commented 2 months ago

@szhorvat this is to prep for part of #1518, prior to my PR there were no direct tests of as_adjacency_matrix().

aviator-app[bot] commented 2 months ago

Current Aviator status

Aviator will automatically update this comment as the status of the PR changes. Comment /aviator refresh to force Aviator to re-examine your PR (or learn about other /aviator commands).

This PR was merged manually (without Aviator). Merging manually can negatively impact the performance of the queue. Consider using Aviator next time.


See the real-time status of this PR on the Aviator webapp.
Use the Aviator Chrome Extension to see the status of your PR within GitHub.
szhorvat commented 2 months ago

You are using randomly generated data, then testing for specific values that depend on the random seed. This is difficult to manage and it's hard to see what goes wrong with failures.

I suggest:

This would make a complete test.

It is important to test self-loops and multi-edges, which is where things go wrong!

szhorvat commented 2 months ago

For example,

dg <- make_graph(c(1,2, 2,1, 2,2, 3,3, 3,3, 3,4, 4,2, 4,2, 4,2), directed=T)
ug <- as.undirected(g, mode="each")

The directed version contains: multi-edges, reciprocal edges, self-loops, multi-self-loops. The undirected version has the same edges, with directions ignored. I chose a 4-vertex graph (instead of a 3-vertex one) to leave room for non-diagonal zeros in the undirected case, and better cover all cases.

Then just add edge weights.

maelle commented 2 months ago

Thank you!!

maelle commented 2 months ago

@szhorvat I've implemented the changes, is this good to go?

szhorvat commented 2 months ago

Based on a quick skim, yes!

aviator-app[bot] commented 2 months ago

This pull request failed to merge: PR cannot be automatically rebased, please rebase manually to continue. After you have resolved the problem, you should remove the blocked pull request label from this PR and then try to re-queue the PR. Note that the pull request will be automatically re-queued if it has the mergequeue label.

Additional debug info: Failed to rebase this PR onto the latest changes from the base branch. You will probably need to rebase this PR manually and resolve conflicts).