Added few tests for /generators/duplication.py and /generators/geomet… #6976
Conversation
…ric.py. Corrected an exception statement in geometric.py (r must be >=0 instead of r must be >= 1)
There was a problem hiding this comment.
Thanks @karthikchiru12 , generally looks good - just a few minor suggestions for further improvements while we're at it!
| with pytest.raises( | ||
| NetworkXError, match=".*n must be greater than or equal to 2" | ||
| ): | ||
| duplication_divergence_graph(1, 1) |
There was a problem hiding this comment.
Super minor suggestion: providing the kwarg + using a non-extreme value makes this test ever so slightly more readable. Not a blocker though!
| duplication_divergence_graph(1, 1) | |
| duplication_divergence_graph(1, p=0.5) |
There was a problem hiding this comment.
@rossbar Added the keyword arguments in places that would improve the readability of the test case.
| def test_invalid_exponent_for_decaying_probability_value(self): | ||
| with pytest.raises(nx.NetworkXException, match=".*r must be >= 0"): | ||
| nx.navigable_small_world_graph(5, p=1, q=0, r=-1, dim=1) | ||
|
|
There was a problem hiding this comment.
+1 for this. I think we should also take this opportunity to test the result when r is between 0 and 1. AFAICT from skimming the reference paper, this is indeed valid, but it'd be nice to have an explicit test (say for r=0.5)!
There was a problem hiding this comment.
@rossbar Added an explicit test case to test the scenario where r is a value between 0 and 1.
There was a problem hiding this comment.
Thanks @karthikchiru12 . Out of curiosity, where did this test case come from? Are we sure that the resulting graph is indeed a valid, navigable small world graph?
There was a problem hiding this comment.
I went ahead and dug a little deeper on this test case and ended up splitting it into two. The original test didn't probe the behavior of r because q=0 means that no long-range edges were being added.
I went ahead and added a second test to probe the general behavior of r - basically larger r should result in fewer long-range edges. This is kind of tricky because it's probabilistic, but as long as 0 < q < number_of_nodes this should hold in general.
There was a problem hiding this comment.
I took the liberty of pushing up a final r < 1 test. It may be overkill since it's probing probabilistic behavior but hopefully it captures the general expected pattern.
Thanks @karthikchiru12
| def test_invalid_exponent_for_decaying_probability_value(self): | ||
| with pytest.raises(nx.NetworkXException, match=".*r must be >= 0"): | ||
| nx.navigable_small_world_graph(5, p=1, q=0, r=-1, dim=1) | ||
|
|
There was a problem hiding this comment.
I went ahead and dug a little deeper on this test case and ended up splitting it into two. The original test didn't probe the behavior of r because q=0 means that no long-range edges were being added.
I went ahead and added a second test to probe the general behavior of r - basically larger r should result in fewer long-range edges. This is kind of tricky because it's probabilistic, but as long as 0 < q < number_of_nodes this should hold in general.
networkx#6976) * Added few tests for /generators/duplication.py and /generators/geometric.py. Corrected an exception statement in geometric.py (r must be >=0 instead of r must be >= 1) * Added kargs to make it more readable. And added few more tests. * STY: Minor signature cleanup. * Update test for r in range [0, 1]. * Add test for general edge scaling with increasing r. --------- Co-authored-by: Ross Barnowski <rossbar@berkeley.edu>
Added a few tests for /generators/duplication.py and /generators/geometric.py. Corrected an exception statement in geometric.py (r must be >=0 instead of r must be >= 1)
This pull request was created as part of the dev-sprints workshop conducted by @navyagarwal on October 2, 2023, as part of Pycon India 2023.