Skip to content

Commit

Permalink
Use kwarg for Theme filename (#5190)
Browse files Browse the repository at this point in the history
There was a recent change on Bokeh branch-2.4 to make the json and filename args to Theme be kwarg-only, but this is incompatible with Dask current usage. I will revert the change in Bokeh to maintain current compatibility, but I am also submitting the change here to set the stage for eventually re-introducing kwarg-only at some point in the future.

Secondarily, I have questions about what to do about testing? This was not caught by the "downstream" tests that Bokeh runs. A simple no-op test that just performs the import would have caught this since the use of Theme is at module scope. It's probably a good idea to make sure all of the bokeh-related modules cleanly import. But all of the bokeh related tests still seem to be in the dask repo, not this one. Should those tests be moved here? Or should I open a separate PR against dask to add tests?

Edit: reverted on Bokeh side at bokeh/bokeh#11484
  • Loading branch information
bryevdv committed Aug 9, 2021
1 parent 0ff6aaf commit b0eefbd
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 5 deletions.
4 changes: 3 additions & 1 deletion distributed/dashboard/components/scheduler.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,9 @@
)
)

BOKEH_THEME = Theme(os.path.join(os.path.dirname(__file__), "..", "theme.yaml"))
BOKEH_THEME = Theme(
filename=os.path.join(os.path.dirname(__file__), "..", "theme.yaml")
)
TICKS_1024 = {"base": 1024, "mantissas": [1, 2, 4, 8, 16, 32, 64, 128, 256, 512]}
XLABEL_ORIENTATION = -math.pi / 9 # slanted downwards 20 degrees

Expand Down
4 changes: 3 additions & 1 deletion distributed/dashboard/components/worker.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,9 @@
)
)

BOKEH_THEME = Theme(os.path.join(os.path.dirname(__file__), "..", "theme.yaml"))
BOKEH_THEME = Theme(
filename=os.path.join(os.path.dirname(__file__), "..", "theme.yaml")
)

template_variables = {"pages": ["status", "system", "profile", "crossfilter"]}

Expand Down
6 changes: 3 additions & 3 deletions distributed/http/scheduler/missing_bokeh.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ class MissingBokeh(RequestHandler):
def get(self):
with log_errors():
self.write(
"<p>Dask needs bokeh >= 0.13.0 for the dashboard.</p>"
"<p>Install with conda: conda install bokeh>=0.13.0</p>"
"<p>Install with pip: pip install bokeh>=0.13.0</p>"
"<p>Dask needs bokeh >= 1.0 for the dashboard.</p>"
"<p>Install with conda: conda install bokeh>=1.0</p>"
"<p>Install with pip: pip install bokeh>=1.0</p>"
)


Expand Down

0 comments on commit b0eefbd

Please sign in to comment.