Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement backend options transformer #653

Merged
merged 62 commits into from
Apr 5, 2022
Merged

Conversation

philippjfr
Copy link
Member

Enables plotting hvPlot output with HoloViews Matplotlib and Plotly backends.

Matplotlib
Plotly

@philippjfr
Copy link
Member Author

philippjfr commented Sep 1, 2021

This is now in a reasonable state but the list of transforms is not at all exhaustive. So as you take over @maximlt here's what needs to be done.

Basically I've transformed some of the most important options but there is a long tail of options which are currently not handled. As a start it would be great if you could go through all user guides and reference notebooks and visually compare the plotly and matplotlib output with the bokeh output. Another useful approach is to insert this print statement:

if opt not in el_opts.allowed_keywords:
    print(f'{opt} not supported')
    ...

and see which options are currently being skipped and either make sure they are also UNSET (if not supported) or define the appropriate transform to ensure they are supported.

@coveralls
Copy link

coveralls commented Sep 1, 2021

Coverage Status

Coverage decreased (-0.5%) to 68.238% when pulling 483fe17 on backend_transformer into 00e1ceb on master.

@maximlt
Copy link
Member

maximlt commented Sep 8, 2021

Here is a small write-up to clarify our intent here. As of now hvplot has a unique backend which is bokeh. hvplot's API allows to pass in kwargs to customize a bokeh plot, on the condition that they are known to holoviews. For instance to plot a dashed line one should provide the bokeh option line_dash="dashed". These options can be found by tab completion or with hvplot.help('line').

hvplot relies obviously on holoviews which itself supports two additional backends: matplotlib and plotly. It seems natural then to add support in hvplot for these alternative backends. There's however code out there that surely relies on customizing plots with bokeh kwargs. Where this happens, hvplot should try its best to transform the bokeh options provided to options understood by the targeted backend. For instance, the line_dash equivalent option for Matplotlib would be linestyle="dashed". Under the hood a mapping is used to store the options that can be linked or not between bokeh and matplotlib/plotly (TBD), and when they can be linked a transform function.

This allows for instance a user to turn a notebook with bokeh plots to a notebook with matplotlib plots, while partly preserving the plot options originally defined for the bokeh backend. Note that this is not a dead-end, the underlying plot object can be accessed with hv.render and further customized.

# These two lines change the backend to matplotlib
import holoviews as hv
hv.extension('matplotlib')

import hvplot.pandas

df = ...
plot = df.hvplot.line(x='x', y='y', line_dash='dashed')
# Display plot in a cell
plot

# Get the Matplotlib figure to further customize it if required
fig = hv.render(plot)
...
fig.savefig('fig.png')

What has not yet been done in this work is to directly deal with matplotlib or plotly options, e.g. what to do with df.hvplot.line(linestyle="dashed"). This could be supported in principle. As of now this would result in a warning being emitted due to an option unknown to hvplot being used.

@maximlt
Copy link
Member

maximlt commented Sep 8, 2021

Here's a list of the current issues observed while running all the notebooks with the PR in its current state.

General:

User guides

Reference

Extras

@maximlt
Copy link
Member

maximlt commented Nov 5, 2021

EDIT 22/03/2022: Fixed

@philippjfr about the issue with line_width not being taken into account, I think I found why but I'm not sure what's the appropriate fix.

At this line

el_options = element.opts.get(backend='bokeh').kwargs

the options I get for a Curve plot when I have declared that I want a Matplotlib plot (import holoviews as hv; hv.extension('matplotlib'); import hvplot.pandas) are:

OrderedDict([('axiswise', False),
             ('color', Cycle()),
             ('framewise', True),
             ('height', 300),
             ('labelled', ['x', 'y']),
             ('line_width', 20),
             ('linewidth', 2),
             ('logx', False),
             ('logy', False),
             ('muted_alpha', 0.2),
             ('responsive', False),
             ('shared_axes', True),
             ('show_grid', False),
             ('show_legend', True),
             ('tools', ['hover']),
             ('width', 700)])

They include linewidth=2, which is the default option for a Curve for the Matplotlib backend in holoviews:
https://github.com/holoviz/holoviews/blob/5e058cadb7c9817216e6eeafb2588ee5c5a5bc8c/holoviews/plotting/mpl/__init__.py#L231

Is it expected that .get() returns options from both backends?

@maximlt
Copy link
Member

maximlt commented Nov 9, 2021

In the Plotting guide the call to create a Hexbin plot (flights.hvplot.hexbin(x='airtime', y='arrdelay', width=600, height=500, logz=True)) doesn't display any image, it just returns :HexTiles [airtime,arrdelay].

Since the following code correctly displays an image with the Matplotlib backend I assume this is a hvplot issue.

hv.HexTiles(flights, kdims=['airtime', 'arrdelay']).opts(fig_size=200, logz=True, colorbar=True)

The object returned by hvplot has this .dataset property:

:Dataset   [index]   (year,month,day,dayofweek,dep_time,crs_dep_time,arr_time,crs_arr_time,carrier,flight_num,tail_num,actual_elapsed_time,crs_elapsed_time,airtime,arrdelay,depdelay,origin,dest,distance,taxi_in,taxi_out,cancelled,cancellation_code,diverted,carrier_delay,weather_delay,nas_delay,security_delay,late_aircraft_delay)

while the one returned by holoviews has:

:Dataset   [airtime,arrdelay]   (year,month,day,dayofweek,dep_time,crs_dep_time,arr_time,crs_arr_time,carrier,flight_num,tail_num,actual_elapsed_time,crs_elapsed_time,depdelay,origin,dest,distance,taxi_in,taxi_out,cancelled,cancellation_code,diverted,carrier_delay,weather_delay,nas_delay,security_delay,late_aircraft_delay)

@maximlt
Copy link
Member

maximlt commented Nov 17, 2021

Executing this:

fig = airports.hvplot.points('Longitude', 'Latitude', geo=True, color='red', alpha=0.2,
                       xlim=(-180, -30), ylim=(0, 72), tiles='ESRI')

works correctly with the Bokeh backend but fails with a horrible error message (holoviz/geoviews#540) with the Matplotlib one.

Interestingly xlim and ylim appear to have been projected to Web Mercartor, while the crs of the Points element is still PlateCarree.

:Overlay
 | Options(axiswise=False, framewise=False)
   .Tiles.I  :Tiles   [x,y]
    | Options(axiswise=False, framewise=False)
   .Points.I :Points   [Longitude,Latitude]
    | Options(alpha=0.2, axiswise=False, cmap='kbc_r', color='red', data_aspect=1, framewise=True,
    |         logx=False, logy=False, marker='o', s=5.477225575051661, show_frame=True, show_grid=False,
    |         show_legend=True, xlim=(-19981848.597392607, -3339584.8351176973), ylim=(0.1113194902619854,
    |         11753184.255101029))

Anyway, Tiles elements don't have a matplotlib implementation in holoviews, so no background tiles would be plotted.

One way to adapt the code to support plotting background tiles with the Matplotlib backend may be to use the WMTS elements provided by geoviews instead of the Tiles elements provided by holoviews, given that the latter have no projection.

@maximlt
Copy link
Member

maximlt commented Feb 22, 2022

Plotly issues:

General

Plotting notebook:

@maximlt maximlt closed this Mar 25, 2022
@maximlt maximlt reopened this Mar 25, 2022
@maximlt
Copy link
Member

maximlt commented Mar 25, 2022

@jlstevens could you please review the last changes wrt compatibility?

hvplot/utilities.py Outdated Show resolved Hide resolved
@jlstevens
Copy link
Collaborator

Other than the suggestion to update the parameter docstring, the latest changes look fine to me!

examples/homepage.ipynb Outdated Show resolved Hide resolved
examples/homepage.ipynb Outdated Show resolved Hide resolved
"x = np.cumsum(np.random.normal(loc=1, scale=5, size=50))\n",
"s = pd.Series(x, name='Time series')\n",
"\n",
"s.hvplot()"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't think this is really related to this PR, but the lack of an x-axis label bothers me. What are the units? Absolute time or presumably time deltas?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the exact same example as pandas' one, which doesn't mean it's perfect!
https://pandas.pydata.org/docs/reference/api/pandas.plotting.lag_plot.html

"cell_type": "markdown",
"metadata": {},
"source": [
"As we discovered in the [Introduction](Introduction.ipynb), HoloViews allows plotting a variety of data types. Here we will use the sample data module and load the pandas and dask hvPlot API:"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would add a sentence along the lines of 'This notebook demonstrates the use of the Matplotlib plotting backend, the equivalent notebook demonstrating the Plotly backend may be found HERE (link)'

"cell_type": "markdown",
"metadata": {},
"source": [
"As we discovered in the [Introduction](Introduction.ipynb), HoloViews allows plotting a variety of data types. Here we will use the sample data module and load the pandas and dask hvPlot API:"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as for matplotlib. One sentence should be enough to link the contents of the notebook to the notebook title.

@jlstevens
Copy link
Collaborator

Made some suggestions for the docs but overall this looks good!

@maximlt
Copy link
Member

maximlt commented Apr 4, 2022

@jlstevens thanks for the review! I applied your suggestions in the last two commits.

@jlstevens
Copy link
Collaborator

Tests look like they are passing and I'm happy to see it merge from my side!

@philippjfr philippjfr merged commit 8a7946d into master Apr 5, 2022
@maximlt maximlt deleted the backend_transformer branch May 8, 2022 03:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants