Helper methods for updating options syntax

[jlstevens]

This PR aims to offer a few useful tools that will make it easier to start using the new options syntax introduced in #3173. Currently the API is quite simple - see the unit tests for examples.

[jlstevens]

Here is the little script I am developing based on this (converter.py):

from holoviews import opts
import pyparsing

while True:
    string = input('> ')
    try:
        options = opts._completer_reprs(string)
    except (SyntaxError, pyparsing.ParseException):
        print('Parse error. Please try again')
        continue

    print('from holoviews import opts')
    print(', '.join(options))
    print('')

It gives you the new import and the completer syntax to use/adapt appropriately. For instance, you can just copy the keywords out if you don't need to qualify by the element.

python converter.py
> %%opts Bivariate [bandwidth=0.5] (cmap='Blues') Points [tools=['box_select']] (size=2)
from holoviews import opts
opts.Bivariate(bandwidth=0.5, cmap='Blues'), opts.Points(size=2, tools=['box_select'])

[jlstevens]

And here is a script to run in examples:

import glob
import sys

import pyparsing
import holoviews as hv
from holoviews import opts, Cycle, Palette
from nbformat.current import read

try:
    import pygments
except:
    pygments = None

hv.extension('bokeh', 'matplotlib', 'plotly')
ns = {'Cycle':Cycle, 'Cycle, Palette':Palette}

for filename in sorted(glob.iglob('**/*.ipynb', recursive=True)):
    if '.ipynb_checkpoints' in filename:
        continue

    with open(filename, 'r') as f:
        nb = read(f, 'json')

    for ws in nb.worksheets:
        for cell in ws.cells:
            if cell['cell_type'] == 'code':
                if '%opts' in cell['input']:
                    for line in cell['input'].splitlines():
                        if '%opts' in line:
                            msg = '\n{filename}:\n     {line}\n'
                            print(msg.format(filename=filename, line=line))

                            try:
                                options = opts._completer_reprs(line, ns=ns)
                            except (SyntaxError, pyparsing.ParseException):
                                print('Unexpected parse error')


                            code = '\n'.join(['from holoviews import opts',
                                              ', '.join(options)])
                            if pygments:
                                style = 'friendly'
                                term = pygments.formatters.Terminal256Formatter(style=style)
                                highlighted = pygments.highlight(code,
                                                        pygments.lexers.PythonLexer(),
                                term)
                                sys.stdout.write(highlighted)
                            else:
                                print(code)

                            while True:
                                string = input('\nContinue? ')
                                if string.strip() in ['y','Y', '']:
                                    break

[jlstevens]

@jbednar @philippjfr The purpose of this PR is to help make the doc update easier. After discussing a few things with Philipp, we decided the API still wasn't quite ready.

In particular we decided we were unhappy with two things:

1. hv.opts being very overloaded....sometimes giving you an Options object (by tab completing kwargs only), sometimes acting like a line magic (string or dictionary arg) or like a cell magic (string or dictionary arg + an object).
2. What to tell people to replace their %opts line magic with. Even though hv.opts already offer a pure Python syntax alternative, it still uses the strings and dictionary formats we are trying to avoid moving forwards.

Commit c40069a address the first point by adding opts.linemagic and opts.cellmagic and directing people to these (or preferably the better alternatives!) from the hv.opts call. The options are to leave the behavior as it is now, warn people or raise an error.

Commit 6985ab5 address the second point adding opts.defaults as a cleaner replacement to the line magic. Instead of %opts Curve (color='red') or hv.opts("Curve (color='red')") (or hv.opts({'Curve': {'style': {'color':'red'}}})) the recommendation would now be:

opts.defaults(opts.Curve(color='red'))

Which I think is less mysterious, cleaner and offers tab-completion.

I'm fairly happy with this though we will need to pick between 'warn', 'error' or leaving as it is and maybe consider whether this is a deprecation that can be managed via hv.config.

[jlstevens]

@philippjfr I have updated five of the bokeh gallery examples in f1654d7 in a way I propose should be idiomatic. I would like to check you agree with this style before I continue.

[jlstevens]

As I work through these examples, I am trying to think of a style guide that describes the rules to apply. This doesn't have to be a style guide we recommend to users (though that would be nice) but it should at least be something we follow in our examples. Here is the provisional set of rules I am thinking of:

1. The .options() method call should normally be used immediately before the object is displayed. In a notebook this means it should appear only at the end of a cell, in a script it might mean it should appear just before rendering. One valid exception to this rule is when the contents of .options varies over a list/dict or set comprehension.
2. Use .options() as few times as possible. If you are building a compositional object, build it and call .options just before display and not at any time prior. In a notebook, you can build a compositional object in stages if you view the pieces between cells, but don't sprinkle .options calls freely within a composite expression.
3. Inline the Options objects from opts.Curve etc into the .options call if possible. Only create a named handle if you need programatic/conditional control (i.e a data structure that isn't static). If you do need such a data structure, it is still preferable to declare it just prior to where it is used in .options and after the data/elements are declared.
4. Don't build a giant expression mixing declaration of elements/objects, composition and options. In notebooks this means the expression before .options should be composed of handles and + and * and nothing else. If necessary make new handles on the composed pieces to satisfy this rule or create a new handle on the overall object (e.g called composition) and call .options on that. This is to avoid large, hard to read and potentially rather scary looking expressions and to help split up the declaration of the data/elements/displayed components from their appearance.
5. These rules apply for compositional objects only. For instance, if you are viewing a single element something like hv.Image(...).options(cmap='jet') is acceptable if the declaration of the element isn't too long. If it is long, declaring a handle is still recommended (e.g im.options(cmap='jet')).
6. If the contents of .options does not fit comfortably in a single line, add a line break immediately afterwards and list the contents with a single extra indent.

[philippjfr]

The "style guide" you laid out sounds good to me and roughly captures what we discussed yesterday.

[philippjfr]

I suppose I should probably use DeprecationWarning or PendingDeprecationWarning in my deprecation PR as well.

[philippjfr]

Not sure I follow the bit about the namespace, also having namespace and ns is confusing me here.

[jlstevens]

Agreed. Here ns is the namespace as an actual namespace dictionary, e.g containing things like Cycle or dim that should be accepted by the parser (if a string is provided). And namespace is the string as the prefix in the repr e.g if you don't want to import opts from holoviews but imported holoviews as hv then you can use namespace='hv' so the reprs are things like 'hv.opts.Curve(...)' that can be executed as reprs.

The problem is that even after explaining it I can't think of better names! Maybe namespace could be namespace_prefix or prefix?

[jbednar]

I'd vote for ns to be ns_dict or namespace_dict, and namespace to be namespace_str or ns_str. But I'm not sure how these objects will be used, so don't take my word for it...

[philippjfr]

I left one comment, otherwise this looks good to me.

[philippjfr]

Now wondering whether we should use warnings.warn(msg, DeprecationWarning) instead.

[jlstevens]

I think this might be a good idea as our param based warnings don't use warnings. Happy to change it if you agree we should make this consistent throughout.

[philippjfr]

I just tested it and I get this horrible thing back:

/Users/philippjfr/miniconda/envs/anacondaviz/lib/python3.6/site-packages/ipykernel_launcher.py:1: DeprecationWarning: Test
  """Entry point for launching an IPython kernel.

So let's not do that. My other preference would be for us to create a singleton Parameterized class in core/util.py which we can call Deprecation or something, which we issue the deprecation warnings on.

[jlstevens]

I added another rule to the 'guide' to capture something I discussed with Philipp:

6. If the contents of .options does not fit comfortably in a single line, add a line break immediately afterwards and list the contents with a single extra indent.

So this:

(density_grid * point_grid).options(opts.Bivariate(bandwidth=0.5, cmap='Blues'), 
                                                          opts.Points(size=2, tools=['box_select']))

Should be:

(density_grid * point_grid).options(
    opts.Bivariate(bandwidth=0.5, cmap='Blues'), 
    opts.Points(size=2, tools=['box_select']))

And even though only one completer is used here:

xticks = ("Friday", "Saturday", "Sunday", "Monday", "Tuesday", "Wednesday", "Thursday")
heatmap.options(opts.HeatMap(radial=True, start_angle=np.pi*19/14, width=600, height=600, 
                             yticks=None, xticks=xticks, xmarks=7, ymarks=3))

It should be:

xticks = ("Friday", "Saturday", "Sunday", "Monday", "Tuesday", "Wednesday", "Thursday")
heatmap.options(
    opts.HeatMap(radial=True, start_angle=np.pi*19/14, width=600, height=600, 
                 yticks=None, xticks=xticks, xmarks=7, ymarks=3))

[jlstevens]

And here is another rule which I think has five parts, three of which I think are optional and two that I think shouldn't be:

1. (optional) In .options(...) try to list element names in alphanumeric order (e.g opts.Curve before opts.HLine)
2. (recommendation) List bare element options before ones with specs (e.g opts.Curve(...) comes before opts.Curve('Ball.Trajectory', ...))
3. (optional) Try to order by specs alphanumerically within an element group (e.g opts.Curve('Ball.Trajectory', ...) comes before opts.Curve('Cannon.Trajectory', ...))
4. (optional) Try to list keywords in alphanumeric order.
5. (recommendation) List container type options last (if present) with opts.Overlay appearing before opts.Layout.

[jlstevens]

Unfortunately, following these rules means updating notebooks that might not even use any magics e.g abe3a09.

[jlstevens]

@philippjfr Please review the changes in my last three commits to see if you agree with my changes. In particular, have a look at how I changed image_range_tool.ipynb to try and conform to the suggested rules. I see very bizarre behavior in that one but I'm not sure whether it is due to my changes or not (sometimes it seems to partially work?)

Note that I seem to have problems with links in general as I couldn't make choropleth_data_link work either using the proposed rules.

[philippjfr]

Changes in the most recent commits look good, I think we have to think about a solution for both Links and Streams. I'd be happy to remap both of them inside the options call to ensure that they stay linked even when you don't use clone=False.

[jbednar]

opts.defaults as a cleaner replacement to the line magic

I like that change; seems clear.

opts.linemagic and opts.cellmagic

I don't understand these objects. They seem to be replacements for magics, not actually magics, so I don't think they should have the word "magic" in their name. Can't you instead name them for what they are, even opts.fromlinemagic and opts.fromcellmagic if that's what they do (convert magic syntax into options objects)? I don't have time to follow them up now, but I do want to register a vote against those names given my current understanding of them...

[jlstevens]

They seem to be replacements for magics, not actually magics, ...

That is true and I am happy to rename them. But it is also true they don't really take line/cell magics as input (directly). Semantically it is something like linemagic_equivalent or linemagic_substitute but those are obviously horrible names.

[jlstevens]

@philippjfr 1bdf12f highlights a number of issues with this example:

1. Having to conda install the matplotlib sample data.
2. yformatter='%d%%' not being propagated to the overlay level.

I hope this is the trickiest example to update as it took a while!

[jlstevens]

One other style to avoid. Instead of:

grid.options('Scatter', tools=['hover', 'box_select'],  fill_alpha=0.2, size=4)

We should now use:

grid.options(
   opts.Scatter(tools=['hover', 'box_select'], fill_alpha=0.2, size=4))

[jbednar]

Why? The first one seems like a very straightforward method call.

[philippjfr]

I don't think we should disallow that style but for consistency in the examples I agree we should go with the latter approach.

[jlstevens]

b47164b shows one valid exception to the rule in lesmis_example: you can use inline .options if it is in a dictionary/list or set comprehension. Not sure about how to handle legend_example nicely yet though...

[jlstevens]

One note I want to make, the reprs of dim are rather confusing. I would like them to be as similar to the original dim expression as possible.

[jlstevens]

I've now updated all the notebooks in the bokeh gallery except for choropleth_data_link.ipynb which has a problem linking.

[jlstevens]

Discussing things with @philippjfr and @jbednar, there is another update to the style guide:

• Options should be specified using .options once, immediately before display, where feasible. There are situations where delaying option setting in this way is not reasonable and you would have to use groups or cycles to disambiguate collections of the same element type. In such situations, the recommendation is to associate the .options call as closely as possible to the element when it is created.

For example, you can use the template styled_element = hv.Element(data, ...).options(...) where the goal is to keep the declaration before .options as short and as readable as possible (e.g data should already be defined as a handle). You should not declare these custom style elements inside larger compositional expressions involving + or * (or containers such as HoloMap).

Note that this new rule actually subsumes the early exception around list/set/dict comprehensions.

[jlstevens]

@philippjfr Rebased and updated the last notebook. I think it is ready for a final review/merge at which point I think I can cut a dev release.

[philippjfr]

Bit confused by these, isn't this opts._cellmagic now?

[jlstevens]

I guess those tests weren't being run!

[philippjfr]

Okay fixed those tests and it's passing now, merging.

[github-actions]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.