-
-
Notifications
You must be signed in to change notification settings - Fork 114
Add New Gallery Examples #1603
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
Add New Gallery Examples #1603
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You may have noticed that we don't store the thumbnails of the current gallery in the repo but on S3 (see the conf.py that links to https://assets.holoviz.org/hvplot/thumbnails). We'll do the same for these new thumbnails.
I'm also not a big fan of generating squared thumbnails by distorting them, they don't look so good, don't they? Minor thing but just wanted to mention it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm all for storing the thumbnails on S3, but the thumbnails do look OK to me in the built docs though. I guess you may look at it differently when you see it in the cards?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the thumbnails do look OK to me in the built docs though
This is where I politely disagree :)
|
I haven't checked but I thought that one nice goal to achieve with the gallery would be to have at least one example per plotting method. I.e. all the plotting methods referenced on this page https://hvplot.holoviz.org/ref/api/index.html (excluding NetworkX method for now). |
Sounds like a good idea. |
|
The idea being that would be a good place for users to find all the types of plots hvPlot can make, and from these examples they can then easily find their reference API for more information. |
Makes sense. I'll try to add a few more as I can. |
* Add coastline * Update doc/gallery/geospatial/geopoints.ipynb Co-authored-by: Azaya <[email protected]> * clean nb * change thumbnail image --------- Co-authored-by: Azaya <[email protected]> Co-authored-by: Isaiah Akorita <[email protected]>
maximlt
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leaving various comments made while reviewing the changes made in this PR. Not expecting any change as I'm making them while I go. But please @Azaya89 have a look at them and answer the questions when you can.
pixi.toml
Outdated
| name = "hvplot" | ||
| channels = ["pyviz/label/dev", "conda-forge"] | ||
| platforms = ["linux-64", "osx-arm64", "osx-64", "win-64"] | ||
| preview = ["pixi-build"] # temp until 0.1.4 release |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was to enable pixi download the dev version of hvsampledata (0.1.4a2) until 0.1.4 is fully released. I think it can be removed now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds strange because we depend on the pyviz/label/dev channel and push hvsampledata dev releases on this channel. I've removed that change.
[feature.doc]
channels = [
# To get dev HoloViews, not always needed.
"pyviz/label/dev",
# To get dev nbsite, not always needed.
"pyviz/label/tooling_dev",
"conda-forge"
]
doc/gallery/index.md
Outdated
| (basic-charts)= | ||
| ## Basic Charts | ||
|
|
||
| ::::{grid} 1 2 2 4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIRC you said you adopted the same approach as Panel. Panel uses a mix of approaches, it uses nbsite's gallery extension for the Component Gallery (https://github.com/holoviz/panel/blob/13419c763105fadc563bc8a6263d39aea33e6041/doc/conf.py#L139) and this custom sphinx-design grid for other pages that include a gallery/grid.
I'm quite tempted to use nbsite's gallery here as it should make the contributor experience a little better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if it makes things better I'm all for it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| # cell execution timeout in seconds (-1 to ignore, 30 by default) | ||
| nb_execution_timeout = 240 | ||
|
|
||
| if os.getenv('HVPLOT_REFERENCE_GALLERY') not in ('False', 'false', '0'): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When we remove pages, we should always add redirect links. In this case, we'll need to keep two sets of redirects:
- links from reference/pandas/foo to reference/tabular/foo
- links from reference/tabular/foo to whatever is the best replacement for this now (plotting API page I think)
doc/gallery/advanced/index.md
Outdated
| <meta http-equiv='refresh' content='0; URL=../index.html#advanced'> | ||
| </head> | ||
|
|
||
| # Advanced Plots |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll see if I can find a better name for this category. Otherwise "Advanced" will end up being a collection of random examples.
| "# Category Breakdown Bar Chart\n", | ||
| "\n", | ||
| "A grouped bar chart that shows how each species of penguin is distributed across different islands. \n", | ||
| "This example uses the `penguins` dataset from hvplot's built-in sample data." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The sample datasets aren't really built-in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will modify
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Already done :)
| "metadata": {}, | ||
| "source": [ | ||
| ":::{note}\n", | ||
| "The `groupby` widget interactivity is only supported in the **Bokeh** backend \n", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not true, this is supported by all the backends.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My assumption was that all interactivity options don't work in the mpl backend. My bad...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HoloViews originally supported Matplotlib only and had already these widgets to drill down a multi-dimensional dataset https://proceedings.scipy.org/articles/Majora-7b98e3ed-00a.pdf.
| "source": [ | ||
| ":::{note}\n", | ||
| "The `groupby` widget interactivity is only supported in the **Bokeh** backend \n", | ||
| "and requires a **live Python environment** (e.g. Jupyter Notebook, JupyterLab, or Panel app).\n", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the docs we use the dynamic=False trick to provide a better user experience.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah but IIRC, I thought we wanted to stop doing that so as not to confuse users since we don't really explain what dynamic=False does in the plot?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True I would like at some point to have a way to deal with that in a better way, with a custom sphinx extension that does the right thing. As it doesn't exist yet so let's stick to dynamic=False :)
| "source": [ | ||
| "import hvplot.xarray # noqa\n", | ||
| "\n", | ||
| "ds = hvplot.sampledata.air_temperature(\"xarray\").sel(time=\"2014-02-25 12:00\")\n", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think using quadmesh on a regular grid like this dataset brings much vs. image?
| "metadata": {}, | ||
| "source": [ | ||
| ":::{note}\n", | ||
| "The `multi_y=True` option enables dual y‑axes **only for the Bokeh backend**. When used with Matplotlib, it falls back to a single shared axis and issues a warning indicating multi‑axis support is unavailable. See [Customizing Plots](https://holoviews.org/user_guide/Customizing_Plots.html#twin-axes).\n", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice to add this example! But the multi_y option should be documented as a HoloViews feature, the example doesn't make that clear.
doc/gallery/interactivity/index.md
Outdated
| Penguins App <penguins_app> | ||
| Sine Wave <sine_curve> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Penguins App and Sine Wave are very similar: 2 widgets, a plot function, and pn.bind.
|
TODO:
|
maximlt
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @Azaya89 !


closes #1611