Added NodeNotFound exceptions to _apply_prediction and simrank, and ignored isolated nodes in panther_similarity
#7110
Conversation
Schefflera-Arboricola
changed the title
NodeNotFound exceptions to _apply_prediction and simrank, and ignored isolated nodes in panther_similarity
Please do! It's generally a good idea to cover these exceptions in tests to make sure you can atleast hit the codeblocks. This may not seem that useful right now, but 10 years from now if someone is refactoring the code it should help them :) |
networkx/algorithms/similarity.py
Outdated
| @@ -1550,6 +1575,10 @@ def panther_similarity( | |||
| """ | |||
| import numpy as np | |||
|
|
|||
| G = G.subgraph( | |||
| [node for node in G.nodes if node not in list(nx.isolates(G))] | |||
| ).copy() | |||
There was a problem hiding this comment.
Is this supposed to silently ignore the isloates? Currently a KeyError pops up if there is a isolate and if there is only one node a ZeroDivisionError: float division by zero pops up.
Code examples:
>>> G = nx.Graph()
>>> G.add_node(1)
>>> nx.panther_similarity(G, 1)
...
...
ZeroDivisionError: float division by zero
>>> G.add_edge(1, 2)
>>> G.add_node(3)
>>> nx.panther_similarity(G, 3)
....
....
KeyError: 3There was a problem hiding this comment.
Thanks for the review! I have raised NetworkXUnfeasible error if the source node is an isolated node and it will also take care of the null graph case. And otherwise it will silently ignore the isolated nodes.
There was a problem hiding this comment.
Another way could be to only allow connected graphs(with more than 1 node) to be passed as G. Please let me know your thoughts on this!
|
Also, here I think we should not remove the source node or initialize k=k+1 and then later remove the source node. Because currently the description of the argument Thank you :) |
… and ignored isolated nodes in `panther_similarity` (networkx#7110) * ignoring isolated nodes in panther_similarity * added NodeNotFound in simrank_similarity * added NodeNotFound in _apply_prediction * bug fix * added Raises sec to all funcs and updated error msg * added Raises sec to simrank_similarity * added tests to test_similarity.py and updated simrank_similarity docs * updated link_prediction.py * updated and added tests in test_link_prediction.py * bug : updated NodeNotFound tests of simrank_similarity * bug : updated test_simrank_target_not_found and style fixes * added NetworkXUnfeasible to panther_similarity * bug fix * added NodeNotFound for panther_similarity * style fix * list() -> set()
closes issue #7108
Please let me know if I should add any tests for these new
NodeNotFoundexceptions or mention them in theRaisessections. I didn't add any because it seemed excessive.Thank you :)