Dask downstream tests and basic usage failures

[bryevdv]

Trying to actually run a simple example with RC4 results in the MissingBokeh route handler page:

Dask needs bokeh >= 2.1.1 for the dashboard.

Install with conda: conda install bokeh>=1.0

Install with pip: pip install bokeh>=2.1.1

But (I think) this happens on any error, so it's unclear what the ultimate error is. cc @pavithraes @jrbourbeau is there a way to get more verbose error information? I am running this in this code in ipython which does not really tell me anything:

from dask.distributed import Client, progress
import dask
import dask.dataframe as dd
from sklearn.linear_model import LinearRegression

client = Client(threads_per_worker=4,
                n_workers=2, memory_limit='2GB')

df = dask.datasets.timeseries()

def train(partition):
    est = LinearRegression()
    est.fit(partition[['x']].values, partition.y.values)
    return est

# client.dashboard_link

# df[['x', 'y']].resample('1h').mean().head()

[bryevdv]

I should also mention that several downstream tests are failing with import errors, e.g:

E   ImportError: cannot import name 'InstanceDefault' from 'bokeh.core.properties' (/usr/share/miniconda3/envs/bk-test/lib/python3.9/site-packages/bokeh/core/properties.py)

ref: https://github.com/bokeh/bokeh/actions/runs/3293338907/jobs/5429833162

However we are not actually testing the correct package. The downstream package install step causes Bokeh 2.4.3 to be installed.

I think we should probably switch to pip-installing downstream dependencies now that we are using a wheel for testing.

[bryevdv]

@mattpap I think this is the breaking change:

E   ImportError: cannot import name 'Panel' from 'bokeh.models' (/usr/share/miniconda3/envs/bk-test/lib/python3.9/site-packages/bokeh/models/__init__.py)

is this a rename? If so we should add a deprecated compatibility alias for now

[bryevdv]

Well, adding an alias will not be sufficient. Doing that, the next problem comes from Dask passing an explicit id

        self.root = figure(
            title="Bytes stored on cluster",
            tools="",
            id="bk-cluster-memory-plot",
            width=int(width / 2),
            name="cluster_memory",
            min_border_bottom=50,
            **kwargs,
        )

then

AttributeError: unexpected attribute 'id' to figure, possible attributes are ...

which is really unfortunate. I don't know that there is any solution that does not involve changes on the Dask end.

[bryevdv]

OK things seem to "work" (modulo my very simple tests) with these changes:

• Panel = TabPanel alias
• kw.pop("id", None) at the top of figure.__init__

However, some plots dont seem to be working and there are console errors:

[bokeh] – "data source has columns of inconsistent lengths"
get_length — bokeh.min.js:264:809

I will open an issue on dask, they should probably add < 3 for now

[jrbourbeau]

Thanks for ping @bryevdv. Looks like I'm a little late to the party. Did you solve the things that needed solving?

I will open an issue on dask, they should probably add < 3 for now

An issue would be great

But (I think) this happens on any error, so it's unclear what the ultimate error is

Yeah, that appears to be the case. It'd be good to fix that.

Also, it looks like we're giving conflicting versions in the

Dask needs bokeh >= 2.1.1 for the dashboard.

Install with conda: conda install bokeh>=1.0

Install with pip: pip install bokeh>=2.1.1

message that's displayed. I've opened up dask/distributed#7172 to improve the situation.

[bryevdv]

@jrbourbeau I think some work is needed on Dask side unfortunately. The "inconsistent lengths" console errors indicate a usage problem that might have been more loosely tolerated in the past.

[bryevdv]

@bokeh/core I propose to delay our release one week to allow dask to release a new version with a Bokeh version cap before we release 3.0

[mattpap]

id="bk-cluster-memory-plot"

This should use name instead, or is there a reason to use id?

[mattpap]

I submitted PR #12505 to completely disallow mixing id with property initializers (as bokehjs already does since serialization/protocol redesign in PR #11960).

[bryevdv]

This should use name instead, or is there a reason to use id?

I'm not sure, I didn't find any reference to the custom id (at least that one) from a quick search of the codbase. I think that setting a custom id used to also set a DOM id? Hopefully it was not being relied on for for that.

[bryevdv]

OK I guess I will close this as a discussion since we are not going to make any changes on our end. I will try to submit a PR this coming week or two to update Dask to work with both 2.x and 3.x (but I do still thing Dask should implement a version cap for their next release cc @jrbourbeau let me know if there is anything I can help with)

[jrbourbeau]

Taking a pass at updating distributed to work with bokeh=3 in dask/distributed#5648. I'm hopefully this will be straightforward-ish. If not then we can definitely add an upper bound for bokeh