Allow `bokeh=3` #5648

jrbourbeau · 2022-01-10T23:44:06Z

…okeh

github-actions · 2022-10-21T18:10:53Z

Unit Test Results

See test report for an extended history of previous test failures. This is useful for diagnosing flaky tests.

      15 files ±    0       15 suites ±0 6h 45m 19s ⏱️ + 15m 50s
  3 218 tests +  49   3 133 ✔️ +  47   84 💤 +1 1 ❌ +1
23 795 runs +347 22 885 ✔️ +338 909 💤 +8 1 ❌ +1

For more details on these failures, see this check.

Results for commit b38f6d9. ± Comparison against base commit a4bfd0e.

♻️ This comment has been updated with latest results.

bryevdv · 2022-10-24T16:48:49Z

distributed/dashboard/components/scheduler.py

+            #     "border": "1px solid lightgray",
+            #     "box-shadow": "inset 1px 0 8px 0 lightgray",
+            #     "overflow": "auto",
+            # },


I think you can now accomplish this similar to:

https://github.com/bokeh/bokeh/blob/branch-3.0/examples/basic/layouts/sizing_mode.py#L19

container.stylesheets.append(":host { border: 10px solid grey; }")

cc @mattpap for the best approach to support both 2.x and 3.x

UIElement.styles is the replacement (works for all UI elements not just markup widgets). The name is styles and not style, so that people don't confuse this with DOM style attribute. I suppose we could have a deprecated alias in markup widgets for some time in 3.x.

Thanks @mattpap! I think 8d8f5f3 should address this one, though let me know if you think I've misunderstood your comment

bryevdv · 2022-10-24T16:51:53Z

distributed/dashboard/components/scheduler.py

@@ -2902,7 +2902,7 @@ def __init__(self, scheduler, **kwargs):
        y_range = Range1d(0, max(self.plugin.nthreads))

        self.root = figure(
-            id="bk-task-group-progress-plot",
+            # id="bk-task-group-progress-plot",


A better alternative that should also work with 2.x is to set name instead of id. AFAIK that ill add a name class to the element that can be used for CSS selection. (cc @mattpap).

Edit: or of these are not being used for anything in particular, then they can just be omitted entirely

Good to know. I tried that initially, but it looks like we're defining a name and id for some figures already (not sure why). I wanted to see if CI breaks if we just don't specify an id at all

alternatively css_classes is still also an option

https://docs.bokeh.org/en/dev-3.0/docs/reference/models/layouts.html#bokeh.models.LayoutDOM.css_classes

@ian-r-rose do you have any insight here?

I took a brief look through the dashboard codebase and didn't see anywhere where the DOM ids are explicitly used, so I think it should be relatively safe to just remove these id= parameters.

I do see a couple of places where bokeh components have custom CSS styling, and that will likely need some updating here. But that is a separate concern from the id parameters here.

I do see a couple of places where bokeh components have custom CSS styling, and that will likely need some updating here

Could you point me to those?

I'm referring to these:

distributed/distributed/http/static/css/base.css

Lines 100 to 107 in 5dccad4

.bk-root .bk-toolbar-box .bk-toolbar-right {

top: 4px;

right: 4px;

}

.bk-root .bk-data-table {

z-index: 0;

}

I'm not sure if they will work with Bokeh 3, but my read of the changelog is that the CSS system has been overhauled, so it may be worth verifying.

@mattpap can you advise?

Due to migration to shadow DOM (or web components, or modularized CSS; different names same idea), styling of bokeh's components with external style sheets is not possible anymore. Each component has to be styled individually. Many CSS classes are gone (e.g. .bk-toolbar-box) or renamed (simplified) (e.g. .bk-toolbar-right becomes .bk-right), because complex selectors are not needed anymore with modularized CSS. .bk-root is gone, because it's not needed anymore. Also the general structure of bokehjs components' DOM and CSS changed significantly, so even if custom CSS is updated to work with shadow DOM, it still will likely not do what's expected without additional changes.

If you have only individual (instances) components to style, then this should work (give or take, I didn't test it):

.bk-root .bk-toolbar-box .bk-toolbar-right { top: 4px; right: 4px; }

Toolbar(stylesheets=[""" :host(.bk-right) { top: 4px; right: 4px; } """])

.bk-root .bk-data-table { z-index: 0; }

DataTable(stylesheets=[""" .bk-data-table { z-index: 0; } """])

Most likely you don't need the second one, as we hopefully fixed all issues with overlapping components.

If you want to style all components of a given kind, like all toolbars, then we don't have a good replacement right now. It's possible to configure stylesheets property via a theme, but we are still working one something more robust. The current advice is that, if you have a lot of custom CSS, then probably you will have to postpone migration to 3.0 for a moment (until 3.0.n or 3.1; in particular in 3.1 we intend to define an "API" for bokehjs' CSS). If you have just a little bit of customization, then we can figure it out on case-by-case basis.

Thanks for the context and code snippets @mattpap!

Many CSS classes are gone (e.g. .bk-toolbar-box) or renamed (simplified) (e.g. .bk-toolbar-right becomes .bk-right)

Grepping through distributed I don't see us using Toolbar object. Maybe this CSS isn't needed anymore?

Most likely you don't need the second one, as we hopefully fixed all issues with overlapping components.

I found that without adding the DataTable CSS I ran into #5136 (the original issue why this custom CSS was added). Thanks again for adding the exact I needed to keep this CSS around for bokeh=3 : )

jrbourbeau · 2022-10-27T15:27:50Z

Thanks for all the feedback @mattpap @bryevdv. Looking at the dashboard with the current state of this PR the only thing that stands out with bokeh=3 is the tabs defined here

distributed/distributed/dashboard/components/scheduler.py

Lines 4311 to 4317 in 9fa952b

    
           tab1 = Panel(child=processing_root, title="Processing") 
        
           tab2 = Panel(child=cpu_root, title="CPU") 
        
           tab3 = Panel(child=occupancy_root, title="Occupancy") 
        
           tab4 = Panel(child=workers_transfer_bytes.root, title="Data Transfer") 
        
           proc_tabs = Tabs(tabs=[tab1, tab2, tab3, tab4], name="processing_tabs") 
        
           doc.add_root(proc_tabs)

are squished vertically (see the tabs in the lower-lefthand part of the screenshots below)

With bokeh=2.4.2:

With bokeh=3.0.0.dev20:

Do either of you have any insight into what might be going wrong here?

bryevdv · 2022-10-27T15:43:58Z

I'm not sure and I'm also afk all today dealing with a family issue. One thing you might try first just in case, is using the latest release candidate (3.0.0rc5)

jrbourbeau · 2022-10-27T17:35:12Z

No worries -- hope all is well.

One thing you might try first just in case, is using the latest release candidate (3.0.0rc5)

Good point. I'm seeing the same thing when I update to bokeh=3.0.0rc5. Maybe @ian-r-rose or @mattpap have thoughts here?

ian-r-rose · 2022-10-27T23:58:59Z

Good point. I'm seeing the same thing when I update to bokeh=3.0.0rc5. Maybe @ian-r-rose or @mattpap have thoughts here?

I am not sure what is going wrong here. In 2.4.x, the Tabs model seems to have its height set to a sensible value, but in 3.0.0rc5 it is not. If I manually set the Tabs(height=700), then things show up as expected (with a fixed height of 700px), but the auto-sizing policy doesn't seem to be working. I don't have a strong grasp of how the auto-sizing policy is intended to work with container elements like Tabs, though I can see that the resulting DOM structures are entirely different between the two versions.

I took a crack at trying to reproduce this outside of the dask dashboard, but was unsuccessful thus far.

bryevdv · 2022-10-28T18:20:14Z

@jrbourbeau just checking in, we do plan to release 3.0 on Monday, in case a version pin is best immediate-term option

jrbourbeau · 2022-10-28T18:39:37Z

Thanks for the update @bryevdv. This PR is really close, but unfortunately it's not quite ready to be merged. I'm going to include dask/dask#9607 and #7219 in the release later today

…okeh

jrbourbeau · 2022-11-08T17:49:19Z

Circling back here after being on PTO last week. I just tried out the changes here with the bokeh=3.0.1 release (landed on conda-forge a few hours ago) and am seeing the same issue described here #5648 (comment). I'll admit I'm a little out of my depth here...@bryevdv any thoughts on how we might be able to resolve the Tabs vertical sizing issue?

bryevdv · 2022-11-08T17:51:13Z

That's probably something @mattpap will have to help with. @mattpap is this is bug, or a usage issue to update?

jrbourbeau · 2022-11-10T20:51:55Z

One additional difference I've noticed is performance reports not taking up the full screen size when using bokeh=3.

With bokeh=2.4.2:

With bokeh=3.0.1:

This probably should be a blocker for distributed, but I did want to surface to the bokeh devs

jrbourbeau · 2022-11-14T16:52:48Z

Okay, I'd like to get this PR in and released sooner rather than later as users are reporting breakages (ImportErrors) when they have bokeh=3 installed. Ideally we'd be able to fix everything, but I'd rather have a couple of plots not look as they should then have distributed code not run.

@fjetter @jacobtomlinson any objections to including this PR in the release later today?

jacobtomlinson

That sounds fine. I expect we will get bug reports that things don't look right but that's better than breakages.

jrbourbeau · 2022-11-14T18:17:32Z

Okay, interesting...we're seeing

AttributeError: module 'importlib' has no attribute 'metadata'

errors in CI. We saw something similar over in dask/dask, which appeared to be related to test-specific code for which we added a workaround (xref dask/dask#9604). Will see if I can reproduce here

bryevdv · 2022-11-14T18:19:21Z

@jrbourbeau that might be this:

Clean up importlib.metadata properly bokeh/bokeh#12588

That is going in a 3.0.2 that I will probably release tonight.

fjetter

I'm with jacob on this. Lifting a pinning is worth a couple of (hopefully temporary) bugs in the visuals

jrbourbeau · 2022-11-14T18:43:38Z

Thanks for pointing to that PR @bryevdv, that certainly looks related. I was able to reproduce the importlib.metadata failure locally -- trying out bokeh=3.0.2.rc1 now to see if that makes the error go away

jrbourbeau · 2022-11-14T18:59:39Z

Okay, bokeh=3.0.2.rc1 did the trick 🎉

We currently have two failures related to the current importlib.metadata situation: distributed/tests/test_init.py::test_version and distributed/cli/tests/test_dask_worker.py::test_single_executable_works.

test_version has a straightforward workaround (see 6dd05a4).

test_single_executable_works is more complicated as it has to do with serializing a utility function that imports pytest, which itself then fails to import importlib.metadata. I'll think a bit about if we can cook up a workaround (if others have suggestions, that would be welcome). Another option would be to delay the release until tomorrow to the bokeh=3.0.2 release will be out and we wouldn't have to worry about working around the importlib.metadata issue

bryevdv · 2022-11-14T20:00:10Z

@jrbourbeau FYI Bokeh 3.0.2 is published to PyPI and the Bokeh channel on anaconda.org (C-F and defaults to follow at their own pace)

jrbourbeau · 2022-11-14T20:22:12Z

Thanks @bryevdv -- thanks was quick! Trying bokeh=3.0.2 out in CI now

jrbourbeau · 2022-11-14T20:38:18Z

C-F and defaults to follow at their own pace

Pushing on conda-forge over in conda-forge/bokeh-feedstock#85

charlesbluca · 2022-11-14T21:08:02Z

rerun tests

This reverts commit d4adb3a. Leaving all code changes, just: * reinstating the `< 3` version restriction * including `< 3` in error messages

jrbourbeau added 2 commits January 7, 2022 13:35

Use development version of bokeh in CI

3d192fb

Merge branch 'main' of https://github.com/dask/distributed into dev-b…

5e9b9b3

…okeh

jrbourbeau mentioned this pull request Jan 10, 2022

Handle Bokeh 3.0 CDSView change #5643

Merged

2 tasks

fjetter mentioned this pull request Apr 7, 2022

How should our test matrix look like? #6085

Open

Merge branch 'main' of https://github.com/dask/distributed into dev-b…

6436877

…okeh

jrbourbeau mentioned this pull request Oct 21, 2022

Dashboard plot sizing issue with bokeh=3 #7173

Closed

jrbourbeau added 4 commits October 21, 2022 13:17

More versions

45477f7

Panel / TabPanel compat

46dbafe

Make GHA yaml valid?

1db6c2f

id and Div compat

7b1056c

jrbourbeau changed the title ~~Test CI against development version of bokeh~~ bokeh=3 compatibility Oct 24, 2022

jrbourbeau mentioned this pull request Oct 24, 2022

Dask downstream tests and basic usage failures bokeh/bokeh#12503

Closed

bryevdv reviewed Oct 24, 2022

View reviewed changes

jrbourbeau self-assigned this Oct 24, 2022

jrbourbeau added 3 commits October 24, 2022 12:58

CDSView filter

0811fec

Div style

8d8f5f3

Lingering id

b98f706

jrbourbeau mentioned this pull request Oct 25, 2022

Release 2022.10.1 dask/community#283

Closed

6 tasks

jrbourbeau added 2 commits October 26, 2022 15:20

Remove ids

3e51931

Handle DataTable CSS

39d3a89

jrbourbeau added 2 commits October 28, 2022 14:02

Merge branch 'main' of https://github.com/dask/distributed into dev-b…

7192b2b

…okeh

Merge branch 'main' of https://github.com/dask/distributed into dev-b…

97fc007

…okeh

Remove bokeh version upper bound

7c9477b

graingert approved these changes Nov 9, 2022

View reviewed changes

jrbourbeau mentioned this pull request Nov 9, 2022

Release 2022.11.0 dask/community#287

Closed

7 tasks

Update env file

3bc45cc

jrbourbeau changed the title ~~bokeh=3 compatibility~~ Allow bokeh=3 Nov 14, 2022

jacobtomlinson reviewed Nov 14, 2022

View reviewed changes

fjetter approved these changes Nov 14, 2022

View reviewed changes

Try importlib.metadata workaround for distributed/tests/test_init.py

6dd05a4

Try bokeh=3.0.2

be63adb

jrbourbeau mentioned this pull request Nov 14, 2022

gpuCI hanging dask/dask#9657

Closed

jrbourbeau mentioned this pull request Nov 14, 2022

Allow bokeh=3 dask/dask#9659

Merged

Remove temporary pip install

b38f6d9

jrbourbeau merged commit d4adb3a into dask:main Nov 15, 2022

jrbourbeau deleted the dev-bokeh branch November 15, 2022 02:13

gjoseph92 mentioned this pull request Nov 17, 2022

Dashboard is wonky with bokeh 3 #7327

Closed

gjoseph92 added a commit to gjoseph92/distributed that referenced this pull request Nov 17, 2022

Partially revert "Allow bokeh=3 (dask#5648)"

04acbff

This reverts commit d4adb3a. Leaving all code changes, just: * reinstating the `< 3` version restriction * including `< 3` in error messages

gjoseph92 mentioned this pull request Nov 17, 2022

Restrict bokeh=3 support #7329

Merged

2 tasks

jrbourbeau mentioned this pull request Mar 31, 2023

Drop id from RMM dashboard component #7739

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow `bokeh=3` #5648

Allow `bokeh=3` #5648

jrbourbeau commented Jan 10, 2022

github-actions bot commented Oct 21, 2022 •

edited

bryevdv Oct 24, 2022 •

edited

mattpap Oct 24, 2022

jrbourbeau Oct 24, 2022

bryevdv Oct 24, 2022 •

edited

jrbourbeau Oct 24, 2022

bryevdv Oct 24, 2022 •

edited

jrbourbeau Oct 25, 2022

ian-r-rose Oct 25, 2022

jrbourbeau Oct 26, 2022

ian-r-rose Oct 26, 2022

bryevdv Oct 26, 2022

mattpap Oct 27, 2022

jrbourbeau Oct 27, 2022

jrbourbeau commented Oct 27, 2022

bryevdv commented Oct 27, 2022

jrbourbeau commented Oct 27, 2022 •

edited

ian-r-rose commented Oct 27, 2022

bryevdv commented Oct 28, 2022

jrbourbeau commented Oct 28, 2022

jrbourbeau commented Nov 8, 2022

bryevdv commented Nov 8, 2022

jrbourbeau commented Nov 10, 2022

jrbourbeau commented Nov 14, 2022

jacobtomlinson left a comment

jrbourbeau commented Nov 14, 2022

bryevdv commented Nov 14, 2022

fjetter left a comment

jrbourbeau commented Nov 14, 2022

jrbourbeau commented Nov 14, 2022

bryevdv commented Nov 14, 2022

jrbourbeau commented Nov 14, 2022

jrbourbeau commented Nov 14, 2022

charlesbluca commented Nov 14, 2022

	.bk-root .bk-toolbar-box .bk-toolbar-right {
	top: 4px;
	right: 4px;
	}

	.bk-root .bk-data-table {
	z-index: 0;
	}

Allow bokeh=3 #5648

Allow bokeh=3 #5648

Conversation

jrbourbeau commented Jan 10, 2022

github-actions bot commented Oct 21, 2022 • edited

Unit Test Results

bryevdv Oct 24, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bryevdv Oct 24, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bryevdv Oct 24, 2022 • edited

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

jrbourbeau commented Oct 27, 2022

bryevdv commented Oct 27, 2022

jrbourbeau commented Oct 27, 2022 • edited

ian-r-rose commented Oct 27, 2022

bryevdv commented Oct 28, 2022

jrbourbeau commented Oct 28, 2022

jrbourbeau commented Nov 8, 2022

bryevdv commented Nov 8, 2022

jrbourbeau commented Nov 10, 2022

jrbourbeau commented Nov 14, 2022

jacobtomlinson left a comment

Choose a reason for hiding this comment

jrbourbeau commented Nov 14, 2022

bryevdv commented Nov 14, 2022

fjetter left a comment

Choose a reason for hiding this comment

jrbourbeau commented Nov 14, 2022

jrbourbeau commented Nov 14, 2022

bryevdv commented Nov 14, 2022

jrbourbeau commented Nov 14, 2022

jrbourbeau commented Nov 14, 2022

charlesbluca commented Nov 14, 2022

Allow `bokeh=3` #5648

Allow `bokeh=3` #5648

github-actions bot commented Oct 21, 2022 •

edited

bryevdv Oct 24, 2022 •

edited

bryevdv Oct 24, 2022 •

edited

bryevdv Oct 24, 2022 •

edited

jrbourbeau commented Oct 27, 2022 •

edited