Magic preprocessors for conversion to vanilla Python #1491

jlstevens · 2017-05-28T22:55:46Z

This is very much a work-in-progress prototype. I will update this description if we decide to go ahead with this approach and after I have pushed forced a cleaner commit history.

jlstevens · 2017-05-29T12:20:45Z

@jbednar @philippjfr Tests are passing so we can now discuss the main change - the refactor of the output magic to allow a new hv.util.output utility to work the same way in pure Python.

The refactor moves almost all the logic to an OutputOptions class so the actual magic class is tiny. An instance of this class is then kept on the Store so it can be used by the output magic and the utility.

I think this is the right thing to do. My major misgiving here is this is not code we like (or are at all proud of!) and I am worried to see this horrible code escape the ipython module where it was safely contained until now.

philippjfr · 2017-05-29T13:24:09Z

holoviews/ipython/__init__.py

-                OutputMagic.allowed['fig'] = list_formats('fig', backend)
-                OutputMagic.allowed['holomap'] = list_formats('holomap', backend)
+
+                Store.output_options = OutputOptions()


Don't quite get this, Store.output_options is an instance but all the actual configuration is set on the class? Can it all be class based?

Maybe although there is this:

def __init__(self, *args, **kwargs): self.shell = kwargs.pop('shell', None) super(OutputOptions, self).__init__(*args, **kwargs) self.output.__func__.__doc__ = self._generate_docstring()

I can try setting shell as a class attribute and removing this, though it will lose the generated docstring.

When would a user every look at the OutputOptions? Seems like internal API to me.

jlstevens · 2017-05-29T14:53:54Z

Switching everything to classmethods seems to have worked, though it does mean the param generated docstring on the magic has been lost.

jbednar · 2017-05-30T00:18:52Z

Sounds great! Are you using the PNG export option from the latest Bokeh, or some other mechanism for Bokeh?

philippjfr · 2017-05-30T00:23:23Z

Are you using the PNG export option from the latest Bokeh?

Yes, see #1493. You can use it in the notebook too but it's slow so not too useful for interactive use.

jbednar · 2017-05-30T01:36:28Z

Ok, great! Looks like this support arrived just in time for what we needed it for!

jlstevens · 2017-06-03T13:59:44Z

Note: We need to allow the opts string syntax (i.e using the parser) everywhere. Including where options can be set for saving/outputting data from the renderers.

jbednar · 2017-06-03T20:41:28Z

Do we have a way that the user could explicitly enable a warning? If so we could suggest that they do so in the release notes, but otherwise not warn for now.

jlstevens · 2017-06-03T20:43:32Z

Do we have a way that the user could explicitly enable a warning?

Right that is another alternative. This is what I mean about wanting a 'config' system - to set things like this.

jlstevens · 2017-06-04T00:12:45Z

@philippjfr I'm expecting the tests to go green and I think this PR should now be reviewed/merged.

It isn't 100% polished or complete but it is probably best to merge sooner rather than later, to help with debugging and to allows us to continue with the docs.

Outstanding work

Here are the things introduced in this PR that need finishing off/fixing. These can be associated with issues as necessary:

examples/README.rst and examples/examples.py are incomplete. I intend for examples.py to generate an index and allow grepping of notebooks referred to in the user guide.
There is a FIXME in one of the notebook preprocessor tests relating to multiple expressions in the line line of a cell.
The parsing of the opts string in the opts method is very rough right now and needs to be made more robust.
The holoviews command/entry point is also quite rough and needs to be improved e.g decent command line help using argparse etc.
Using hv.util.opts and hv.util.output in the docs - as our docs are being rewritten, we must make sure to use these now!
Improve validation in hv.util.opts to match the magic.

Edit: Related ideas

I think the idea of hv.config and a load_backend utility (used by the notebook_extension) should be added to the new utils subpackage in separate PRs.
In theory we could have an additional (optional) preprocessor to wrap expressions at the ends of all cells is something like show (e.g pop up a window for matplotlib). With this output side-effect the hv.util.output might make sense when replacing a cell magic...

Although there are a few loose bits and pieces, the bulk of what this PR offers is the hv.util.opts and hv.util.output helpers that can be used from pure Python instead of the magics. E.g hv.util.output(fig='svg') will work as will the string version hv.util.output("fig='svg'"). The opts utility only accepts the string specifications used by the magic.

In addition, this PR adds a collection of preprocessors that make use of these utils to convert notebooks to simple .py scripts. One of the main things this will enable is launching notebooks as bokeh apps with minimal (if any!) changes.

philippjfr · 2017-06-04T11:36:47Z

holoviews/__init__.py

@@ -12,7 +12,7 @@
                            commit="$Format:%h$", reponame='holoviews')

 from .core import archive                                # noqa (API import)
-from .core.dimension import OrderedDict, Dimension       # noqa (API import)
+from .core.dimension import OrderedDict, Dimension, Dimensioned       # noqa (API import)


How come we need Dimensioned in the top-level namespace?

Thanks! That is a stray/unnecessary import now - I still need Dimensioned in util.__init__.py though...

Fixed in f5f12c2

philippjfr · 2017-06-04T11:38:18Z

holoviews/core/dimension.py

+            except:
+                options = OptsSpec.parse(
+                    '{clsname} {options}'.format(clsname=self.__class__.__name__,
+                                                 options=options))


Seems okay for now but it might be better to list an explicit exception.

Sure. This falls under my bullet points for something to improve. The exception is part of it, but the whole try/except here isn't a very robust approach. I can specify an exception now but this whole thing needs improving (in another PR imho).

Specified SyntaxError in a325041

philippjfr · 2017-06-04T11:40:17Z

holoviews/util/__init__.py

+    Cell magic version acts as a no-op.
+    """
+    if obj is not None:
+        return obj


Did we want to allow this in a later PR? I remember discussing that all it has to do is temporarily set the renderer options.

Could you outline how you can imagine that working? Even if I set the options state temporarily for an expression, it will have no effect unless that expression has some sort of side-effect, namely output. The cell magic resets everything after display and this concept of 'display' doesn't exist outside the notebook.

If you do have ideas about this, I would like to hear them as I would like these utilities to correspond to the magics as closely as possible...

Good point probably doesn't make sense afterall.

I added a suggestion above regarding a potential show call that could be injected by an optional preprocessor. Something to think about (maybe it isn't worth it?) and definitely not for this PR.

philippjfr · 2017-06-04T11:42:11Z

Looks good, as you laid out in your summary there's some more work to be done here but the PR looks to be in a mergeable state. Happy to merge when you tell me.

jlstevens · 2017-06-04T12:19:03Z

@philippjfr Let's merge once the tests are green.

philippjfr · 2017-06-04T12:49:07Z

Great, merging.

jlstevens · 2017-06-04T19:50:43Z

I've added another item to the list above regarding validation. We need to make sure we address all the things in that list.

jlstevens added the status: WIP label May 28, 2017

jlstevens force-pushed the opts_preprocessor_prototype branch 3 times, most recently from afa7fcb to 727431b Compare May 29, 2017 10:46

philippjfr reviewed May 29, 2017

View reviewed changes

jlstevens force-pushed the opts_preprocessor_prototype branch 4 times, most recently from bb3346e to fe57fa2 Compare May 29, 2017 20:01

jlstevens mentioned this pull request May 29, 2017

Added thumbnailer.py to generate thumbnail PNGs ioam/ioam-builder#4

Merged

jlstevens added 15 commits June 1, 2017 01:12

Opts magic conversion prototype

3f645bb

Fix to only apply to cells using %%opts

b7264ff

Moved opts utility to util

eb273c4

Refactored magic replacement preprocessor

f02c959

Added support for replacing line magics

48dbfe3

Refactored OutputMagic into OutputControl

86af334

Defined hv.util.output helper

a1bb4e1

Improved usability of wrap_cell_expression

6f6dda3

Fixed where Store.output_control is set

b291d63

Refactored magics preprocessor into separate classes

5fde8ba

Switched opts utility argument order

642f8e7

The hv.util.opts utility can now also replace the line magic

3f3e560

OptsMagicProcessor can now convert the %opts line magic

31fe462

Defined cell magic version of hv.util.output as a no-op

59750c1

Renamed OutputControl to OutputOptions

ca68c67

jlstevens added 3 commits June 3, 2017 14:38

Updated argument order in OutputMagicProcessor

7d920dc

Fixed setting of OutputMagic docstring for Python 3

c71b009

Restored TestOutputMagic unit tests

b2b86df

jlstevens added 6 commits June 3, 2017 15:25

Removed warning when using __call__

746db40

Renamed testutils.py to testcoreutils.py

29ce705

Added 4 unit tests of the hv.util.output utility

33589d6

Added extra hv.util.output unit test and four tests for hv.util.opts

d60f78d

Mirrored settings reset in setUp

9b4e250

Updated TestOptsUtil tests

c9009f4

jlstevens force-pushed the opts_preprocessor_prototype branch from c016375 to c9009f4 Compare June 3, 2017 19:53

Updated options reset for opts utility tests

98eecbb

jlstevens force-pushed the opts_preprocessor_prototype branch from d1d066d to 98eecbb Compare June 3, 2017 21:21

jlstevens added 3 commits June 4, 2017 00:28

Removed Conversion_Example notebook used to test prototype

9cfbfe0

Added unit tests of the notebook preprocessors

d4054fb

Added docstrings to new Preprocessor classes

80ad114

philippjfr reviewed Jun 4, 2017

View reviewed changes

jlstevens added 2 commits June 4, 2017 13:03

Removed unnecessary import

f5f12c2

Opts method now only catches SyntaxError when parsing

a325041

philippjfr merged commit 91a0917 into master Jun 4, 2017

philippjfr deleted the opts_preprocessor_prototype branch June 17, 2017 17:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Magic preprocessors for conversion to vanilla Python #1491

Magic preprocessors for conversion to vanilla Python #1491

jlstevens commented May 28, 2017

jlstevens commented May 29, 2017

philippjfr May 29, 2017

jlstevens May 29, 2017 •

edited

Loading

philippjfr May 29, 2017

jlstevens commented May 29, 2017

jbednar commented May 30, 2017

philippjfr commented May 30, 2017

jbednar commented May 30, 2017

jlstevens commented Jun 3, 2017

jbednar commented Jun 3, 2017

jlstevens commented Jun 3, 2017

jlstevens commented Jun 4, 2017 •

edited

Loading

philippjfr Jun 4, 2017

jlstevens Jun 4, 2017

jlstevens Jun 4, 2017

philippjfr Jun 4, 2017

jlstevens Jun 4, 2017

jlstevens Jun 4, 2017

philippjfr Jun 4, 2017

jlstevens Jun 4, 2017

philippjfr Jun 4, 2017

jlstevens Jun 4, 2017

philippjfr commented Jun 4, 2017

jlstevens commented Jun 4, 2017

philippjfr commented Jun 4, 2017

jlstevens commented Jun 4, 2017

Magic preprocessors for conversion to vanilla Python #1491

Magic preprocessors for conversion to vanilla Python #1491

Conversation

jlstevens commented May 28, 2017

jlstevens commented May 29, 2017

Choose a reason for hiding this comment

jlstevens May 29, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jlstevens commented May 29, 2017

jbednar commented May 30, 2017

philippjfr commented May 30, 2017

jbednar commented May 30, 2017

jlstevens commented Jun 3, 2017

jbednar commented Jun 3, 2017

jlstevens commented Jun 3, 2017

jlstevens commented Jun 4, 2017 • edited Loading

Outstanding work

Edit: Related ideas

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

philippjfr commented Jun 4, 2017

jlstevens commented Jun 4, 2017

philippjfr commented Jun 4, 2017

jlstevens commented Jun 4, 2017

jlstevens May 29, 2017 •

edited

Loading

jlstevens commented Jun 4, 2017 •

edited

Loading