The from_dataset function was doing a truthiness check on the graph object, instead of specifically testing for Noneness. This meant that our tests and behaviors around enriching already populated graph objects was working fine, but it also meant that we would fail in situations where a fresh nx.Graph or nx.DiGraph object were to be passed in instead - and fail by ignoring them and creating a new graph entirely.
This would be very subtle to find, since most of our tests around around undirected graphs and we wouldn't notice an empty nx.Graph not being used in favor of another empty nx.Graph (that was then populated). The problem manifests much more clearly when trying to populate a directed graph instead and receiving an undirected graph in return!
In addition to fixing the bug, I added a backwards compatible flag (is_directed=False) to from_file such that users of directed graphs don't have to go the whole way to the full blown from_dataset just to use a directed graph. The current behavior is maintained in that it defaults to an undirected graph, but users can now specifically request creation of a directed graph instead.
The
from_datasetfunction was doing a truthiness check on the graph object, instead of specifically testing for Noneness. This meant that our tests and behaviors around enriching already populated graph objects was working fine, but it also meant that we would fail in situations where a fresh nx.Graph or nx.DiGraph object were to be passed in instead - and fail by ignoring them and creating a new graph entirely.This would be very subtle to find, since most of our tests around around undirected graphs and we wouldn't notice an empty nx.Graph not being used in favor of another empty nx.Graph (that was then populated). The problem manifests much more clearly when trying to populate a directed graph instead and receiving an undirected graph in return!
In addition to fixing the bug, I added a backwards compatible flag (
is_directed=False)tofrom_filesuch that users of directed graphs don't have to go the whole way to the full blownfrom_datasetjust to use a directed graph. The current behavior is maintained in that it defaults to an undirected graph, but users can now specifically request creation of a directed graph instead.Closes #29