examples icon indicating copy to clipboard operation
examples copied to clipboard

attractors: Modernize_attractors

Open Azaya89 opened this issue 2 years ago • 3 comments


Modernizing an example checklist

Preliminary checks

  • [x] Look for open PRs and issues that reference the project you are updating. It is possible previous unmerged work in PR could be re-used to modernize the project. Comment on these PRs and issues when appropriate, hopefully we should be able to close some of them after your modernizing work.

Change ‘anaconda-project.yml’ to use the latest workable version of packages

  • [x] Pin python=3.11
  • [x] Remove the upper pin (e.g. hvplot<0.9 to hvplot, panel>=0.12,<1.0 to panel>=0.12) of all other dependencies. Removing the upper pins of dependencies could necessitate code revisions in the notebooks to address any errors encountered in the updated environment. Should complexities or extensive time requirements arise, document issues for team discussion on whether to re-pin specific packages or explore other solutions.
  • [x] Add/update the lower pin of all other dependencies (e.g. hvplot to hvplot>=0.9.2, hvplot>=0.8 to hvplot>=0.9.2). Usually, the new/updated lower pin of a dependency will be the version resolved after anaconda prepare has been run. Execute !conda list in a notebook, or anaconda run conda list in the terminal, to display the version of each dependency installed in the environment. Adjusting the lower pin helps ensure that the locks produced for each platform (linux-64, win-64, osx-64, osx-arm64) rely on the tested dependencies and not on some older versions.
  • [x] If one of the channels include conda-forge or pyviz, ask Maxime if it can be removed

Plot API updates (discussed on a per-example basis)

  • [x] Generally, try to replace HoloViews usage with hvPlot. At a certain point of complexity, such as with the use of ‘.select’, it might be better to stick with HoloViews. Additional examples of ‘complexity boundaries’ should be documented in this document.
  • [x] Almost always, try to replace the use of datashade with rasterize (read this page). Essentially, rasterize allows Bokeh to handle the colormapping instead of Datashader.

Interactivity API updates (discussed on a per-example basis)

  • [x] Remove all pn.interact usage
  • [x] Avoid .param.watch() usage. This is pretty low-level and verbose approach and should not be used in Examples unless required, or an Example is specifically trying to demo its usage in an advanced workflow.
  • [x] Prefer using pn.bind(). Read this page for explanation.
  • [x] For apps built using a class approach, when they create a view() method and call it directly, update the class by inheriting from pn.viewable.Viewer and replace view() by __panel__(). Here is an example.

Panel App updates (discussed on a per-example basis)

  • [x] If the project doesn’t at any point create a Panel app at all, consider creating one. It can be as simple as wrapping a plot in pn.Column, or more complicated to incorporate widgets, etc. Make the final app .servable().
  • [x] If the project creates an app in a notebook but doesn’t deploy it (i.e. there is no command: dashboard declaration in the anaconda-project.yml file), try adding it.
  • [x] If the project already deploys an app but doesn’t wrap it in a nice template, consider wrapping it in a template.
  • [x] If the project deploys an app wrapped in a template, customize the template a little so all the apps don’t look similar (e.g. change the header background color). This doesn’t need to be discussed.
  • [x] Comment start If you are building the application in a single cell, you can construct a template explicitly, like template = pn.template.BootstrampTemplate, but if building up an app across multiple cells, it is probably cleaner to declare the template at the top with pn.extension(template='bootstrap'). See how to guide on setting a template.

General code quality updates

  • [x] If the notebook disables warnings (e.g. with warnings.simplefilter(‘ignore’) somewhere at the start of the notebook, remove this line. Try to update the code to remove the warnings, if any. If updating the code to remove the warnings is taking significant amount of time and effort, bring it up for discussion and we may decide to disable warnings again.

Text content

  • [x] Edit the text content anywhere and everywhere that it can be improved for clarity.
  • [x] Check the links are valid, and update old links (e.g. http -> https, xyz.pyviz.org -> xyz.holoviz.org)
  • [x] Remove instructions to install packages inside an example

Visual appearance - Example

  • [x] Check that the titles/headings make sense and are succinct.
  • [x] Check that the text content blocks are easily readable; revise into additional paragraphs if needed.
  • [x] Check that the code blocks are easily readable; revise as needed. (e.g. add spaces after commas in a list if there are none, wrap long lines, etc.)
  • [x] Check image and plot sizes. If possible, making them responsive is highly recommended.
  • [x] Check the appearance on a smartphone (check Google to see how to adapt the appearance of your browser to display pages as if they were seen from a smartphone, this is usually done via the web developer tools). This is not a top priority for all examples, but if there are a few easy and straightforward changes to make that can improve the experience, let’s do it.
  • [x] Check the updated notebook with the original notebook

Visual appearance - Gallery

  • [x] Check the thumbnail is visually appealing
  • [x] Check the project title is well formatted (e.g. Ml Annotators to ML Annotators), if not, add/update the examples_config.title field in anaconda-project.yml
  • [x] Check the project description is appropriate, if not, update the description field in anaconda-project.yml

Workflow (after you have made the changes above)

  • [x] Run successfully doit validate:<projectname>
  • [x] Run successfully doit test:<projectname>
  • [x] Run successfully doit doc_one –name <projectname>. It’s better if the project notebook(s) is saved with its outputs (but be sure to clear outputs before committing to the examples repo!) when building the docs. Then open this file in your browser ./builtdocs/index.html and check how the site looks.
  • [x] If you’re happy with all the above, open a PR. Reminder, clear notebook outputs before pushing to the PR.

Azaya89 avatar Apr 16 '24 12:04 Azaya89

Your changes were successfully integrated in the dev site, make sure to review the pages of the projects you touched before merging this PR: https://holoviz-dev.github.io/examples/. You can also download an archive of the site from the workflow summary page which comes in handy when your dev site built was overriden by another PR (we have a single dev site!).

github-actions[bot] avatar Apr 16 '24 12:04 github-actions[bot]

Your changes were successfully integrated in the dev site, make sure to review the pages of the projects you touched before merging this PR: https://holoviz-dev.github.io/examples/. You can also download an archive of the site from the workflow summary page which comes in handy when your dev site built was overriden by another PR (we have a single dev site!).

github-actions[bot] avatar Apr 16 '24 15:04 github-actions[bot]

I'm currently reviewing this PR but wanted to write a message dedicated to the issue we observed in attractors_panel.ipynb and the widgets pane containing some weird things.

How it should look: image

vs. how it looks at the moment in this PR:

image

We can see that:

  • the Attractors section with the different buttons is missing, replaced instead by the Parameterized UI of a single Attractor
  • the expanded layout in the new version is slightly shifted to the right, while in the original version, everything is well aligned.

Let's look at the original code (before this PR):

diff --git a/before.py b/pr.py
index ee25674..85bc4fe 100644
--- a/before.py
+++ b/pr.py
@@ -2,14 +2,13 @@ import param, panel as pn
 from panel.pane import LaTeX
 pn.extension('katex')
 
-class Attractors(param.Parameterized):
-    attractor_type = param.ObjectSelector(params.attractors["Clifford"],
-                                          params.attractors, precedence=0.9)
+class Attractors(pn.viewable.Viewer):
+    attractor_type = param.Selector(objects=params.attractors, default=params.attractors["Clifford"], precedence=0.9)
 
-    parameters = param.ObjectSelector(params, precedence=-0.5, readonly=True)
+    parameters = param.Selector(objects=params.attractors, precedence=-0.5, readonly=True)
 
-    plot_type = param.ObjectSelector(
-        "points", precedence=0.8, objects=['points', 'line'],
+    plot_type = param.Selector(
+        precedence=0.8, objects=['points', 'line'],
         doc="Type of aggregation to use"
     )
 
@@ -23,7 +22,7 @@ class Attractors(param.Parameterized):
             self.param.set_param(attractor_type=a)
 
     @param.depends("attractor_type.param", "plot_type", "n")
-    def view(self):
+    def __panel__(self):
         return datashade(self.attractor_type(n=self.n), self.plot_type,
                          palette[self.attractor_type.colormap][::-1])

The change made to the parameters definition looks suspicious, going from parameters = param.ObjectSelector(params, ...) to parameters = param.Selector(objects=params.attractors, ...). objects should be set to params, not params.attractors. So I made that change locally and now get this error:

TypeError: 'ParameterSets' object is not iterable

params is a ParameterSets instance (a class defined in attractors.py), it's not of the type list or dict that the SelectorParameter expects (getting the same error with ObjectSelector by the way). I'm not sure why the original code used an ObjectSelector. In the old code, the app looks identical if I replace it with params = param.Parameter(objects, ...). However, if I apply the same change in the PR, I see two problems:

  1. The section name is not Attractors but ParameterSets00207.
  2. The buttons in the expanded section have varying widths

image

On these issues:

  1. Param issue https://github.com/holoviz/param/issues/937 for which I have a potential fix.
  2. Panel issue I guess 😃 , perhaps related to the weird alignment issue already noted. I haven't looked into that yet.

And this is what I get when applying the potential fix in Param.

image


EDIT after quickly looking at the Panel issue. I think it has to do with changes in the default margin of the Param pane and changes in the default width of buttons. Running this before creating the Param pane can help making the app looking closer to how it used to be:

pn.Param.margin = 0
# pn.widgets.Button.width = 300  # why doesn't this work?
pn.widgets.Button.param.width.default = 300

image

maximlt avatar Apr 19 '24 16:04 maximlt

Your changes were successfully integrated in the dev site, make sure to review the pages of the projects you touched before merging this PR: https://holoviz-dev.github.io/examples/. You can also download an archive of the site from the workflow summary page which comes in handy when your dev site built was overriden by another PR (we have a single dev site!).

github-actions[bot] avatar Apr 26 '24 20:04 github-actions[bot]

Your changes were successfully integrated in the dev site, make sure to review the pages of the projects you touched before merging this PR: https://holoviz-dev.github.io/examples/. You can also download an archive of the site from the workflow summary page which comes in handy when your dev site built was overriden by another PR (we have a single dev site!).

github-actions[bot] avatar Apr 27 '24 09:04 github-actions[bot]

Looks good, thanks! There seem to be some changes referenced but not yet in the PR, so there may be a version still on @Azaya89 's machine that needs to be pushed here.

I have pushed the updated changes now. However, I'm still not done with the last bit concerning the widgets panel in the attractors_panel.ipynb notebook.

Azaya89 avatar Apr 27 '24 09:04 Azaya89

@Azaya89 this rebase went wrong https://github.com/holoviz-topics/examples/pull/381/commits/561469837ef14cb299555f4565334d64875de90b.

I suggest you revert it, and just copy/paste the changes from #200, it's sometimes the easier way.

maximlt avatar May 23 '24 21:05 maximlt

@Azaya89 this rebase went wrong 5614698.

I suggest you revert it, and just copy/paste the changes from #200, it's sometimes the easier way.

Yeah, it was just the merge conflicts in the dodo.py and build_one.yml files I didn't notice earlier. I've corrected them now.

Azaya89 avatar May 23 '24 22:05 Azaya89

Your changes were successfully integrated in the dev site, make sure to review the pages of the projects you touched before merging this PR: https://holoviz-dev.github.io/examples/. You can also download an archive of the site from the workflow summary page which comes in handy when your dev site built was overriden by another PR (we have a single dev site!).

github-actions[bot] avatar May 23 '24 22:05 github-actions[bot]

Done!

Azaya89 avatar May 24 '24 08:05 Azaya89

Your changes were successfully integrated in the dev site, make sure to review the pages of the projects you touched before merging this PR: https://holoviz-dev.github.io/examples/. You can also download an archive of the site from the workflow summary page which comes in handy when your dev site built was overriden by another PR (we have a single dev site!).

github-actions[bot] avatar May 24 '24 09:05 github-actions[bot]