Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added few tests for /generators/duplication.py and /generators/geomet… #6976

Merged
merged 5 commits into from
Nov 8, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion networkx/generators/geometric.py
Original file line number Diff line number Diff line change
Expand Up @@ -681,7 +681,7 @@ def navigable_small_world_graph(n, p=1, q=1, r=2, dim=2, seed=None):
if q < 0:
raise nx.NetworkXException("q must be >= 0")
if r < 0:
raise nx.NetworkXException("r must be >= 1")
raise nx.NetworkXException("r must be >= 0")

G = nx.DiGraph()
nodes = list(product(range(n), repeat=dim))
Expand Down
6 changes: 6 additions & 0 deletions networkx/generators/tests/test_duplication.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,12 @@ def test_probability_too_small(self):
with pytest.raises(NetworkXError):
duplication_divergence_graph(3, -1)

def test_minimum_desired_nodes(self):
with pytest.raises(
NetworkXError, match=".*n must be greater than or equal to 2"
):
duplication_divergence_graph(1, 1)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super minor suggestion: providing the kwarg + using a non-extreme value makes this test ever so slightly more readable. Not a blocker though!

Suggested change
duplication_divergence_graph(1, 1)
duplication_divergence_graph(1, p=0.5)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rossbar Added the keyword arguments in places that would improve the readability of the test case.



class TestPartialDuplicationGraph:
"""Unit tests for the
Expand Down
12 changes: 12 additions & 0 deletions networkx/generators/tests/test_geometric.py
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,18 @@ def test_navigable_small_world(self):
gg = nx.grid_graph([5]).to_directed()
assert nx.is_isomorphic(G, gg)

def test_invalid_diameter_value(self):
with pytest.raises(nx.NetworkXException, match=".*p must be >= 1"):
nx.navigable_small_world_graph(5, p=0, q=0, dim=1)

def test_invalid_long_range_connections_value(self):
with pytest.raises(nx.NetworkXException, match=".*q must be >= 0"):
nx.navigable_small_world_graph(5, p=1, q=-1, dim=1)

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)

Comment on lines +260 to +263
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+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)!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rossbar Added an explicit test case to test the scenario where r is a value between 0 and 1.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


class TestThresholdedRandomGeometricGraph:
"""Unit tests for :func:`~networkx.thresholded_random_geometric_graph`"""
Expand Down