TST: clean up isomorphism tests#8364
Conversation
beb5203 to
58179b8
Compare
|
It takes 0.84s on my machine and one of the tree_isomorphism tests takes longer. I guess I could remove that mark. It is much longer than tests where the monomorphism is found. It has to search through the whole tree instead of stopping when it finds something. :) Anyway, I took out the mark. And I "turned on" two other kinda slow tests that I had commented out a few lines above this spot. All 3 are tests that are slow due to show it is not isomorphic. Made all the other changes too. Thanks for those suggestions!! |
Peiffap
left a comment
There was a problem hiding this comment.
LGTM. I make a lot of these refactoring PRs so I owe you a few reviews back! :)
Most of these comments are nits, and some of them might go against your goal of not "changing" anything. None of them are particularly strong recommendations.
I think we've talked before about the setup_x functions and I still believe we should be moving away from them in the long term, but that's far beyond the scope of this change.
| def test_subgraph(self): | ||
| g1 = nx.Graph() | ||
| g2 = nx.Graph() | ||
| g1.add_edges_from(self.g1edges) | ||
| g2.add_edges_from(self.g2edges) | ||
| g1 = nx.Graph(self.g1edges) | ||
| g2 = nx.Graph(self.g2edges) | ||
| g3 = g2.subgraph([1, 2, 3, 4]) | ||
| gm = iso.GraphMatcher(g1, g3) | ||
| assert not gm.is_isomorphic() | ||
| assert gm.subgraph_is_isomorphic() | ||
| assert gm.subgraph_is_monomorphic() | ||
|
|
||
| def test_directed_subgraph(self): | ||
| g1 = nx.DiGraph(self.g1edges) | ||
| g2 = nx.DiGraph(self.g1edges) | ||
| g3 = g2.subgraph(["a", "b", "c", "d"]) | ||
| gm = iso.DiGraphMatcher(g1, g3) | ||
| assert not gm.is_isomorphic() | ||
| assert gm.subgraph_is_isomorphic() | ||
| assert gm.subgraph_is_monomorphic() |
… double read, fix pics
dschult
left a comment
There was a problem hiding this comment.
Thanks for these suggestions @Peiffap
I agree with all of them. It is hard to know where to stop sometimes with this sort of thing. They certainly need more work, but I am hoping I've got enough from this exercise to pull hem together nicely once the different algorithms have the same supported features (subgraph iso and mono).
Clean up existing isomorphism tests.
Add tests where there are comments saying something is missing.
Move two vf2pp MultiDiGraph tests from vf2 test module to vf2pp test module.
Remove some test code in an if/else clause that can't be reached.
Add tests of MultiDiGraph class in
test_vf2pp.py.Move adding edges into the constructors to ease reading.
Added graph to the parametrization so can remove two test methods.
Drew a few more ascii pictures of test graphs cuz it helps me a lot to see it.
I stayed away from wholesale changes here -- just cleaning up existing stuff.
An upcoming PR will add "common" tests for all algorithm types, exposing parts of the api that are not provided yet.
The plan is to then work on providing e.g. subgraph iso handling in vf2pp and subgraph mono in both vf2pp and ismags.
After all that I will revisit the tests to see whether we can rely on the "common" tests while removing duplicate or unneeded tests from the separated test modules.