{"action":"created","issue":{"url":"https://api.github.com/repos/networkx/networkx/issues/2272","repository_url":"https://api.github.com/repos/networkx/networkx","labels_url":"https://api.github.com/repos/networkx/networkx/issues/2272/labels{/name}","comments_url":"https://api.github.com/repos/networkx/networkx/issues/2272/comments","events_url":"https://api.github.com/repos/networkx/networkx/issues/2272/events","html_url":"https://github.com/networkx/networkx/pull/2272","id":181080321,"number":2272,"title":"Make PyDot Support Great Again","user":{"login":"leycec","id":217028,"avatar_url":"https://avatars.githubusercontent.com/u/217028?v=3","gravatar_id":"","url":"https://api.github.com/users/leycec","html_url":"https://github.com/leycec","followers_url":"https://api.github.com/users/leycec/followers","following_url":"https://api.github.com/users/leycec/following{/other_user}","gists_url":"https://api.github.com/users/leycec/gists{/gist_id}","starred_url":"https://api.github.com/users/leycec/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/leycec/subscriptions","organizations_url":"https://api.github.com/users/leycec/orgs","repos_url":"https://api.github.com/users/leycec/repos","events_url":"https://api.github.com/users/leycec/events{/privacy}","received_events_url":"https://api.github.com/users/leycec/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":3,"created_at":"2016-10-05T07:12:58Z","updated_at":"2016-10-06T18:31:59Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/networkx/networkx/pulls/2272","html_url":"https://github.com/networkx/networkx/pull/2272","diff_url":"https://github.com/networkx/networkx/pull/2272.diff","patch_url":"https://github.com/networkx/networkx/pull/2272.patch"},"body":"This pull request refactors the `networkx.drawing.nx_pydot` submodule to support the well-maintained [`pydot`](https://github.com/erocarrera/pydot) project in lieu of the effectively dead [`pydotplus`](https://github.com/carlos-jenkins/pydotplus) fork, resolving [issue #2235](https://github.com/networkx/networkx/issues/2235) (_Abandon All pydotplus, Ye Who Graph Here_) and restoring basic sanity.\r\n\r\nSee #2235 for full-frontal details. A brute-force **tl;dr** follows:\r\n\r\n* The last commit for [`pydotplus`](https://github.com/carlos-jenkins/pydotplus) was at **[December 8th, 2014](https://github.com/carlos-jenkins/pydotplus/commits/master?page=1)** – nearly two years ago. Long-standing open issues and pull requests remain unresolved. `pydotplus` is a face-rotting zombie.\r\n* The last commit for [`pydot`](https://github.com/erocarrera/pydot) was at **[July 2nd, 2016](https://github.com/erocarrera/pydot/commits/master)** – only a little over two months ago. Issues and pull requests are quickly dispatched with, as evidenced by a [closed issue](https://github.com/erocarrera/pydot/issues/133) I foolishly opened two days ago.\r\n\r\n**`pydot` lives.**\r\n\r\n## Not all that Glitters is PyDot Gold\r\n\r\n`pydot` 1.2.0 recently broke [backward API compatibility](https://github.com/erocarrera/pydot/blob/master/ChangeLog) with pre-1.0.29 forks (like `pydotplus`) in mostly beneficial ways. Let's talk a bit about that.\r\n\r\n### Dot File Deserialization Gone Wild\r\n\r\nIn `pydot` 1.2.0, the `pydot.graph_from_dot_data()` function now _unconditionally_ returns a list of all deserialized `pydot.Dot` objects rather than _conditionally_ returning a list if the passed data contains two or more graphs or a single `pydot.Dot` object otherwise.\r\n\r\nThis is a positive change. The prior approach complicated caller handling and encouraged caller errors. It didn't help that the prior approach was poorly documented and that everyone downstream made simplistic assumptions about that approach. How do we know this?\r\n\r\nBecause NetworkX currently assumes the `pydot.graph_from_dot_data()` function to _always_ return a single `pydot.Dot` object. Which was never the case. Maybe nobody cares about `pydot` integration with dot files containing multiple graphs? It is pretty edge-case. <sup>_Or maybe my tenuous deathgrip on sanity has slipped another notch lower._</sup>\r\n\r\nNetworkX functionality calling the `pydot.graph_from_dot_data()` function includes:\r\n\r\n* `networkx.drawing.nx_pydot.read_dot()`.\r\n* `networkx.drawing.nx_pydot.pydot_layout()`.\r\n* `networkx.drawing.tests.test_pydot.TestPydot.pydot_checks()`.\r\n\r\nThis functionality has all been refactored to accept lists of `pydot.Dot` objects. In the case of both `pydot_layout()` and `pydot_checks()`, a list of exactly one object is _always_ returned. Life is simple here.\r\n\r\nIn the case of `read_dot()`, however, a list of two or more objects are returned if the passed dot file contains two or more graphs. This was _always_ the case, of course. NetworkX currently ignores this discrepancy by returning \"sight-unseen\" whatever `pydot` returns _without_ documenting that a list of graphs rather than a single graph might be returned.\r\n\r\nTwo options exist here:\r\n\r\n1. **Break NetworkX backwards compatibility.** Refactor `read_dot()` to unconditionally return a list of `pydot.Dot` objects – just like the underlying `pydot.graph_from_dot_data()`. Orthogonality is preserved, which is mildly good. All downstream logic is broken, which is strongly bad.\r\n1. **Mostly preserve NetworkX backwards compatibility.** Refactor `read_dot()` to unconditionally return only a single `pydot.Dot` object if the passed dot file contains only one graph _or_ raise an exception otherwise.\r\n\r\nUnder the safe assumption that 99.9999% of all calls to `read_dot()` pass dot files containing only one graph, the latter _\"Don't break everything!\"_ approach is arguably the sane approach. I like sanity, so I went with that. \r\n\r\nIf anyone ever requires deserialization of multiple `pydot.Dot` objects from a single dot file, subsequently adding a new NetworkX function (e.g., `read_dots()`) _or_ a new optional keyword argument to the existing `read_dot()` function (e.g., `return_list=False`) would be trivial. This is left as an exercise to the reader.\r\n\r\n### String Decoding and What It Can Do for You\r\n\r\nIn `pydot` 1.2.0,<sup>..._maybe earlier?_</sup> the `pydot.Dot.create_dot()` method now returns an encoded byte array (`bytes` in Python 3) rather than a probably-incorrectly-decoded Unicode string (`str` in Python 3).\r\n\r\nThis is a positive change, too. The prior approach masked inevitable encoding errors with a Jedi handwave: _\"These are the character-like bytes you might be looking for. No further questions!\"_\r\n\r\nThis does require, however, that NetworkX functionality calling `create_dot()` explicitly decode its return value into a proper Unicode string. My simplistic assumption is that GraphViz encodes this return value via the current user's preferred locale (e.g., `LC_*` environment variables on POSIX-compatible platforms), which is safely decodable by NetworkX via the same encoding. This assumption appears to hold. May the [Spaghetti Monster](https://s-media-cache-ak0.pinimg.com/564x/66/68/18/6668181a59aaf6846cfcb90f2a780e18.jpg) preserve us all.\r\n\r\nThe `pydot.Dot.create_dot()` method is only called by the `networkx.drawing.nx_pydot.pydot_layout()`,  which has been refactored to decode the returned byte array via the current `locale.getpreferredencoding()`. <sup>..._which is probably just `utf-8`, in most cases._</sup>\r\n\r\n## We Can Build a Better PyDot Integration\r\n\r\nThe newer 1.2.0 `pydot` API is a non-trivial departure from the older pre-1.0.29 `pydot` API. Since both cannot be reasonably supported at the same time, this pull request revises all `networkx.drawing.nx_pydot` functions to explicitly require `pydot >= 1.2.0`. To enforce this constraint in a human-readable manner:\r\n\r\n* A new private `networkx.drawing.nx_pydot._import_pydot()` function has been defined, importing and returning the `pydot` module if the currently installed version of that module is `>= 1.2.0` _or_ raising an `ImportError` exception. For [robustness](https://stackoverflow.com/a/21065570/2809027), the ubiquitous `pkg_resources.parse_version()` function  is called to implement this version comparison. Since NetworkX requires setuptools at installation time, the setuptools-bundled `pkg_resources` package is assumed to be available at runtime. This assumption is almost certainly safe for most definitions of \"almost,\" \"certainly,\" and \"safe.\"\r\n* Most `import pydotplus` statements in the codebase now read `pydot = _import_pydot()`.\r\n\r\n## Are You Quite Done Yet?\r\n\r\n**Nope.** Unit tests have also been improved to explicitly test the `networkx.drawing.nx_pydot.pydot_layout()` function. Because this is a safety-first pull request. For maintainability, I also obsessively documented everything in these tests. Because obsessiveness.\r\n\r\nThanks for the hard volunteerism, fellow NetworkX-ers! This graph paper is for you. :chart_with_upwards_trend: "},"comment":{"url":"https://api.github.com/repos/networkx/networkx/issues/comments/252049569","html_url":"https://github.com/networkx/networkx/pull/2272#issuecomment-252049569","issue_url":"https://api.github.com/repos/networkx/networkx/issues/2272","id":252049569,"user":{"login":"MridulS","id":5363860,"avatar_url":"https://avatars.githubusercontent.com/u/5363860?v=3","gravatar_id":"","url":"https://api.github.com/users/MridulS","html_url":"https://github.com/MridulS","followers_url":"https://api.github.com/users/MridulS/followers","following_url":"https://api.github.com/users/MridulS/following{/other_user}","gists_url":"https://api.github.com/users/MridulS/gists{/gist_id}","starred_url":"https://api.github.com/users/MridulS/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/MridulS/subscriptions","organizations_url":"https://api.github.com/users/MridulS/orgs","repos_url":"https://api.github.com/users/MridulS/repos","events_url":"https://api.github.com/users/MridulS/events{/privacy}","received_events_url":"https://api.github.com/users/MridulS/received_events","type":"User","site_admin":false},"created_at":"2016-10-06T18:31:59Z","updated_at":"2016-10-06T18:31:59Z","body":"@leycec That was fast! 🍻  Thanks for debugging pydot, was scratching my head for a long time to find it! \r\n\r\n"}}
{"actor":{"display_login":"MridulS"}}