Graph generators take 2 #26
Conversation
Added an ifmain block that runs the unit tests. This enables running the tests via `python3 infra/graph_generators_test.py`.
- Refactors the helper functions for bidirectionalizing and reversing graphs, so that they do not mutate the input graphs but rather return new graphs. (Mutating function arguments is a bad smell in general, and should be avoided when not necessary.) - Create a new helper base test class with assertions for graph isomorphism and that a graph is isomorphic to a provided iterator of edges. - Write more extensive tests for the bidirectional and reverse functions. Test plan: `python3 infra/graph_generators_test.py`
|
I'm currently working on reviewing this PR. Rather than adding comments and asking for changes, I'm going to try pushing updates directly to the branch. Please see some here: 4b123e4 |
| return output | ||
|
|
||
|
|
||
| def set_types(graph, node_types="vanilla", edge_types="vanilla"): |
There was a problem hiding this comment.
I don't like this API (or the way that types are handled in this PR generally). When node_types is provided as a dict, there's no guarantee that every node is present in the dict, so I expect that often times nodes will go without any type at all. So there's no guarantee that types are consistently present. Also, AFAIK we don't have any code yet that actually consumes node types.
Can we strip out the node and edge types concept from this PR and merge without them? Then, once we actually need node types, we can put them back in, but we will know a little more at that time based on what the actual usage is.
wchargin
force-pushed
the
graph-generators-take-2
branch
from
4b123e4 to
22aed74
Compare
Simple Graph Generators for Infra
Initially in graph-generators-for-testing branch, the code fell behind, I created a new branch based on master to avoid conflicts.
Then I worked through the code review from the original pull request here:
#21
New version has formatting updated to match python convention per suggestions from @decentralion, includes factoring out helper methods, error handling and tests for both helper methods and error handling.