Implementation of Bokeh Chart Custom Component #3

sfc-gh-kmcgrady · 2025-01-17T20:30:11Z

This PR intrroduces the bokeh chart in its entirety for release.

We are deliberately releasing 3.6.1 in order to test out the auto update functionality.

The component is simple avoiding react and making it a pure JS app.

Recommendation to Reviewers: Feel free to give format suggestions, though the next PR #5 focuses on the formatting fixes.

sfc-gh-braethlein · 2025-01-20T09:27:37Z

setup.py

+
+setuptools.setup(
+    name="streamlit-bokeh",
+    version="3.6.1",


just sanity-check: was your idea to follow the bokeh versioning? So when bokeh is updated, we also bump this version to the same one?

That's the current plan. But I am reconsidering it now if there's a issue and we need to issue a patch. Let me think a little bit more.

Yeah good point, I think it might make sense to use our own versioning. Not only for patches, but perhaps we change something with the (internal) API or whatever and then we might run into issues. So decoupling the versions might make sense

I think we could try to match only the major and minor versions of bokeh and keep the patch version for ourselves.

I like that. Will adjust to 3.6.0

sfc-gh-braethlein · 2025-01-20T09:29:32Z

setup.py

+        # By definition, a Custom Component depends on Streamlit.
+        # If your component has other Python dependencies, list
+        # them here.
+        "streamlit >= 0.63",


I am wondering whether we should be way more strict. While technical apparently possible, would we really want to enable such an old version to work?

It would help with adoptability, but we can also try doing 1.26.0+ for partners' sake?

sounds good!

sfc-gh-braethlein · 2025-01-20T09:35:13Z

streamlit_bokeh/frontend/public/fonts/SourceSansPro-Bold.woff2

We cannot re-use the font files that already come bundled with Streamlit because we are in an iframe, can we?

The answer is yes unfortunately. We could hard code the parent frame's URL which enables caching, but that would be more brittle should Streamlit adjust the location and naming.

Too bad! I agree, the alternative sounds too brittle!

sfc-gh-braethlein · 2025-01-20T09:36:54Z

streamlit_bokeh/frontend/public/index.html

+<!DOCTYPE html>
+<html lang="en">
+  <head>
+    <title>Streamlit Component</title>


nit: better title might a "Streamlit Bokeh Chart"

sfc-gh-braethlein · 2025-01-20T09:39:26Z

streamlit_bokeh/frontend/src/index.ts

+    // embed_item is actually an async function call, so a race condition
+    // can occur if updateChart is called twice, leading to two Bokeh charts
+    // to be embedded at the same time.


but this cannot happen here because we use await? Should we have a lock or something indicating that the function was called?

Ah yes, we can probably remove the comment now.

sfc-gh-braethlein · 2025-01-20T09:46:00Z

streamlit_bokeh/frontend/src/index.ts

+  const renderedAppTheme = JSON.stringify(renderData.theme)
+
+  let themeChanged = false
+  if (
+    renderData.args.bokeh_theme !== currentTheme ||
+    appTheme !== renderedAppTheme
+  ) {
+    currentTheme = renderData.args.bokeh_theme


Could you elaborate a little bit please why there are two ways of passing a theme to the chart? renderData.args.bokeh_theme seems to come from the Python side, right? Where does renderData.themecome from?

renderData.theme is the theme that comes from Streamlit (so if I switch light theme to dark theme) renderData.theme changes. I probably should make this more efficient and specify if the bokeh theme changed or the bokeh theme is "streamlit" and the rendered theme has changed. I'll fix that.

sfc-gh-braethlein · 2025-01-20T09:48:45Z

streamlit_bokeh/frontend/src/index.ts

+    if (currentTheme === "streamlit") {
+      use_theme(streamlitTheme(renderData.theme as Theme))
+      themeChanged = true
+    } else if (currentTheme in window.Bokeh.Themes) {
+      use_theme(window.Bokeh.Themes[currentTheme])
+    } else {
+      use_theme(streamlitTheme(renderData.theme as Theme))
+      themeChanged = true
+    }


I would change this to:

if (currentTheme === "streamlit" || !(currentTheme in window.Bokeh.Themes)) { use_theme(streamlitTheme(renderData.theme as Theme)) themeChanged = true } else { use_theme(window.Bokeh.Themes[currentTheme]) }

In my opinion this makes it more obvious that the first and third case perform the identical operation.

sfc-gh-braethlein · 2025-01-20T09:52:16Z

streamlit_bokeh/frontend/src/index.ts

+  // Python script.
+
+  // We tell Streamlit to update our frameHeight after each render event, in
+  // case it has changed. (This isn't strictly necessary for the example


still, why not move it into the if(hasChanged || themeChanged) block?

sfc-gh-braethlein · 2025-01-20T09:56:09Z

streamlit_bokeh/frontend/src/index.ts

+  if (
+    renderData.args.bokeh_theme !== currentTheme ||
+    appTheme !== renderedAppTheme
+  ) {


nit: it might be cleaner to move the theme logic to its own function so that onRender is more dense

sfc-gh-braethlein · 2025-01-20T09:56:36Z

streamlit_bokeh/frontend/src/react-app-env.d.ts

Can the file be removed?

Also, was the idea to not use React to have one dependency less we have to maintain?

I'll check. It came from the template, so I assumed it was necessary. And yes. Since Bokeh doesn't come in a nice react component, and we do minimal processing. I figured it was simpler to just use vanilla JS. Should be faster.

sfc-gh-braethlein · 2025-01-20T09:58:04Z

streamlit_bokeh/frontend/src/streamlit-theme.ts

+}
+
+export function streamlitTheme(theme: Theme): BokehTheme {
+  // @ts-expect-error fadedText10 is not defined in the Theme type


its not defined, but set by our streamlit-component-lib? In this case, instead of expecting this error, perhaps this should be fixed in the Theme of streamlit-component-lib instead.

I'll double check with @sfc-gh-lmasuch since this is tied to the advanced theming (also a note to make sure the advanced theming is included in the minimum version we require.

oh yeah, checking the minimal version is a good aspect!

Yeah, I think we eventually need to officially expose colors that are generated from theme variables as well to custom components. But not sure if fadedText10 will be something that stays forever. Maybe its a bit safer to actually create this color manually here: transparentize(textColor, 0.8). fadedText10 is something that could be removed at any time -> which probably will break this custom component

sfc-gh-braethlein · 2025-01-20T09:59:46Z

streamlit_bokeh/frontend/tsconfig.json

+    "resolveJsonModule": true,
+    "isolatedModules": true,
+    "noEmit": true,
+    "jsx": "react"


we don't seem to use React in this repo

Will remove.

sfc-gh-braethlein

How do you feel about adding a couple of unit tests for the functions in index.ts (getChartData (and the fact that it is memoized), an extracted processTheme function, ...) ? I know you have added e2e tests in #6, but for fast and general testing it could make test.

sfc-gh-lmasuch · 2025-01-21T15:15:04Z

streamlit_bokeh/__init__.py

+if TYPE_CHECKING:
+    from bokeh.plotting.figure import Figure
+
+ST_BOKEH_VERSION = importlib.metadata.version("streamlit_bokeh")


nit: should we also expose this under __version__ here?

Will make this version. Though with the new convention, I opted to hard code the dependency version for simplicity. The update script will update everything.

sfc-gh-lmasuch · 2025-01-21T15:16:05Z

streamlit_bokeh/__init__.py

+
+ST_BOKEH_VERSION = importlib.metadata.version("streamlit_bokeh")
+
+def streamlit_bokeh(figure: "Figure", use_container_width=True, theme: str = "streamlit", key=None):


the types of use_container_width and key + the return type is missing here

sfc-gh-lmasuch · 2025-01-21T15:45:34Z

setup.py

+    packages=setuptools.find_packages(),
+    include_package_data=True,
+    classifiers=[],
+    python_requires=">=3.10",


nit: is it also possible to support python 3.9 since this is also still supported with Streamlit?

sfc-gh-lmasuch · 2025-01-21T15:46:26Z

README.md

@@ -1,2 +1,3 @@
-# streamlit-bokeh-chart
+# streamlit-bokeh
+


nit: we might want to add a bit more info here, but that's probably a follow up or something to get Debbie involved in.

sfc-gh-lmasuch · 2025-01-21T15:52:34Z

streamlit_bokeh/frontend/src/index.ts

+// See https://github.com/bokeh/bokeh/blob/3.6.2/bokehjs/src/lib/models/plots/plot.ts#L203
+const DEFAULT_WIDTH = 600 // px
+const DEFAULT_HEIGHT = 600 // px


nit: The default width & height we use for vega charts is 400 x 350 (not sure about plotly). No strong opinion, but maybe it makes sense to use similar numbers for bokeh.

sfc-gh-lmasuch

LGTM 👍 added a couple of minor comments.

sfc-gh-lmasuch · 2025-01-22T12:28:47Z

Two changes that might be worth doing:

Add types for streamlit_bokeh
Use transparentize(textColor, 0.8) instead of fadedText10 to avoid breakage.

Initial workings of Bokeh Chart

5e55e69

sfc-gh-kmcgrady enabled auto-merge January 17, 2025 20:46

sfc-gh-kmcgrady disabled auto-merge January 17, 2025 22:07

sfc-gh-kmcgrady force-pushed the feature/bokeh_chart branch from c2b0c60 to 5e55e69 Compare January 17, 2025 22:08

sfc-gh-kmcgrady and others added 3 commits January 17, 2025 17:43

Fix outline transparency

5bc6bbd

Clean up code

5b78fe3

Delete streamlit_bokeh/frontend/.prettierrc

a2255ee

sfc-gh-braethlein reviewed Jan 20, 2025

View reviewed changes

sfc-gh-lmasuch reviewed Jan 21, 2025

View reviewed changes

sfc-gh-lmasuch approved these changes Jan 21, 2025

View reviewed changes

sfc-gh-braethlein approved these changes Jan 21, 2025

View reviewed changes

sfc-gh-kmcgrady added 3 commits January 21, 2025 17:10

Address comments

2b32067

Add js tests workflow

302b794

Fix yml formatting

9ada0ee

sfc-gh-kmcgrady added 4 commits January 21, 2025 17:16

Add nvmrc

3759123

JS -> TS

f1be88e

Address comments

484275e

Fix test

9fd3986

Add Precommit to repo and update formatting (#5)

5629fd7

sfc-gh-kmcgrady merged commit 590b9a4 into main Jan 22, 2025
4 checks passed

sfc-gh-kmcgrady deleted the feature/bokeh_chart branch January 22, 2025 18:48


		ST_BOKEH_VERSION = importlib.metadata.version("streamlit_bokeh")

		def streamlit_bokeh(figure: "Figure", use_container_width=True, theme: str = "streamlit", key=None):

Implementation of Bokeh Chart Custom Component #3

Implementation of Bokeh Chart Custom Component #3

Uh oh!

Conversation

sfc-gh-kmcgrady commented Jan 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sfc-gh-braethlein Jan 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sfc-gh-braethlein Jan 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sfc-gh-kmcgrady Jan 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sfc-gh-braethlein Jan 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sfc-gh-lmasuch Jan 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sfc-gh-braethlein left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sfc-gh-lmasuch Jan 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

sfc-gh-kmcgrady commented Jan 17, 2025 •

edited

Loading

sfc-gh-braethlein Jan 20, 2025 •

edited

Loading

sfc-gh-braethlein Jan 20, 2025 •

edited

Loading

sfc-gh-kmcgrady Jan 20, 2025 •

edited

Loading

sfc-gh-braethlein Jan 20, 2025 •

edited

Loading

sfc-gh-lmasuch Jan 21, 2025 •

edited

Loading

sfc-gh-lmasuch Jan 21, 2025 •

edited

Loading

sfc-gh-lmasuch commented Jan 22, 2025 •

edited

Loading