search

cjdrake / ipython-magic

IPython Magic Functions
BSD 2-Clause "Simplified" License
16 stars 7 forks source link

extend to allow for specification of layout engine #1

Closed jbn closed 9 years ago

jbn commented 9 years ago

I tried to operate on your code unobtrusively, patching only the rundot function. Granted, this means my bit of code is always called. But, given how you used rundot, it also gave me a seamless, single point of extension.

Dot is rarely the layout engine I use. Basically, this entire patch exists so i can do:

%%dot -k engine ...

where engine is one of neato, circo, twopi, fdp, or sfdp. I didn't change the default. It is still dot.

My code is not specific to the -k argument. It assumes everything before valid dot language code is a command line argument.

cjdrake commented 9 years ago

@jbn, thanks for the patch. I need to look up some background on this matter, b/c I'm not terribly familiar with the other layout engines.

jbn commented 9 years ago

Here is the gist of the layout engines (har!). Note, the graph is the same in each cell. Only the layout engine changes. I use the same "-K engine" argument that you would use for dot on the command line. But, if you were just using the CLI, you'd use "circo", "neato", etc instead of "dot".

cjdrake commented 9 years ago

Great example. Thanks :)

cjdrake commented 9 years ago

Haven't merged this yet b/c I was busy w/ SciPy, and I don't have a test setup. Will evaluate by Monday.

cjdrake commented 9 years ago

I love the intent of this pull request, and we should definitely support this capability.

I am not sure about the user interface.

First off, I think there's a potential security hole here. This magic function is using the shell to do this transformation. So something like " -K rm -rf * " becomes possible. Also you can probably cause other problems such as changing the output format and other error conditions.

In a more ideal world, it might be nice to have something like this:

In [1]: %dot(layout='dot') digraph { ... }

This way, you can control the options that you give to dot. Much more robust and secure, and doesn't rely on regular expressions to glean info from a text prefix.

Since IPython doesn't seem to support arguments to magic functions, maybe we could propose an alternative that meets your requirements. You said the patch exists so you can do "dot -K engine ...". So how about we create new magic functions for the various layout engines.

So the new proposal would be to add the following line/cell magics: %neato, %twopi, %circo, %fdp, %sfdp, %osage, %patchwork, ...

This would be a relatively easy change. Probably change rundot(s) to rundot(s, layout='dot'), and do some copy-paste of current dot magic to include all the new names.

What do you think?

jbn commented 9 years ago

Sorry. Finishing up my dissertation proposal.

I agree on all counts. Sometime this weekend, I'll resubmit aligned with your review.