marimo-team / marimo

A reactive notebook for Python — run reproducible experiments, execute as a script, deploy as an app, and version with git.
https://marimo.io
Apache License 2.0
7.95k stars 278 forks source link

Altair Chart with `.selection_point` not working when the Altair chart is a layering of two charts. #2901

Closed ztawilvdl closed 23 minutes ago

ztawilvdl commented 2 days ago

Describe the bug

When trying to create an mo.ui.altair_chart using a layered Altair chart that has a selection_point attached, we get an error of:

Unrecognized signal name: "select_point_0"
Error: Unrecognized signal name: "select_point_0" at LW (http://localhost:2718/assets/index-S77OjPaJ.js:49:305728) at EM.getSignal (http://localhost:2718/assets/VegaLite-Bg5TTIWG.js:11:414153) at http://localhost:2718/assets/VegaLite-Bg5TTIWG.js:11:341918 at Array.forEach (<anonymous>) at EA (http://localhost:2718/assets/VegaLite-Bg5TTIWG.js:11:341880) at lS (http://localhost:2718/assets/VegaLite-Bg5TTIWG.js:11:374350) at sS (http://localhost:2718/assets/VegaLite-Bg5TTIWG.js:11:374078) at aS (http://localhost:2718/assets/VegaLite-Bg5TTIWG.js:11:373982) at IC (http://localhost:2718/assets/VegaLite-Bg5TTIWG.js:11:391142) at http://localhost:2718/assets/VegaLite-Bg5TTIWG.js:11:409045

However, when removing one of the layers it works. Or when removing the selection_point from the chart creation it works.

Environment

{
  "marimo": "0.9.20",
  "OS": "Darwin",
  "OS Version": "23.5.0",
  "Processor": "arm",
  "Python Version": "3.10.13",
  "Binaries": {
    "Browser": "131.0.6778.71",
    "Node": "v22.3.0"
  },
  "Dependencies": {
    "click": "8.1.3",
    "docutils": "0.16",
    "itsdangerous": "2.1.2",
    "jedi": "0.18.2",
    "markdown": "3.5.2",
    "narwhals": "1.12.1",
    "packaging": "24.1",
    "psutil": "5.9.4",
    "pygments": "2.14.0",
    "pymdown-extensions": "10.7",
    "pyyaml": "6.0.1",
    "ruff": "0.4.10",
    "starlette": "0.37.2",
    "tomlkit": "0.12.3",
    "typing-extensions": "4.12.2",
    "uvicorn": "0.27.1",
    "websockets": "12.0"
  },
  "Optional Dependencies": {
    "altair": "5.4.1",
    "anywidget": "0.9.13",
    "duckdb": "1.0.0",
    "pandas": "1.5.3",
    "polars": "1.4.1",
    "pyarrow": "17.0.0"
  }
}

Code to reproduce

import marimo as mo
import pandas as pd
import altair as alt

This will cause the error

test_counts = pd.DataFrame([
    {"Level1": "a", "count": 1, "stage": "france"},
    {"Level1": "b", "count": 2, "stage": "france"},
    {"Level1": "c", "count": 3, "stage": "england"}
])

_base = alt.Chart(test_counts)

_point = alt.selection_point(encodings=['x'])
_chart = (
    _base.mark_bar().encode(
        x=alt.X('Level1').sort(order="descending").title("Subpillar"),
        y=alt.Y('count').title("Number of Companies"),
        color=alt.condition(_point, "stage", alt.value('lightgray')),
    )
    .add_params(_point)
)

_rule = _base.mark_rule(strokeDash=[2, 2]).encode(
    y=alt.datum(2),
    color=alt.datum("england")
)

# This will cause error
_mo_chart = mo.ui.altair_chart((alt.layer(_chart, _rule)))
# _mo_chart = mo.ui.altair_chart((_chart))
_mo_chart
# alt.layer(_chart, _rule)

This works

test_counts = pd.DataFrame([
    {"Level1": "a", "count": 1, "stage": "france"},
    {"Level1": "b", "count": 2, "stage": "france"},
    {"Level1": "c", "count": 3, "stage": "england"}
])

_base = alt.Chart(test_counts)

_point = alt.selection_point(encodings=['x'])
_chart = (
    _base.mark_bar().encode(
        x=alt.X('Level1').sort(order="descending").title("Subpillar"),
        y=alt.Y('count').title("Number of Companies"),
        color=alt.condition(_point, "stage", alt.value('lightgray')),
    )
    .add_params(_point)
)

_rule = _base.mark_rule(strokeDash=[2, 2]).encode(
    y=alt.datum(2),
    color=alt.datum("england")
)

# This will cause error
# _mo_chart = mo.ui.altair_chart((alt.layer(_chart, _rule)))
_mo_chart = mo.ui.altair_chart((_chart))
_mo_chart
# alt.layer(_chart, _rule)

This also works

test_counts = pd.DataFrame([
    {"Level1": "a", "count": 1, "stage": "france"},
    {"Level1": "b", "count": 2, "stage": "france"},
    {"Level1": "c", "count": 3, "stage": "england"}
])

_base = alt.Chart(test_counts)

_point = alt.selection_point(encodings=['x'])
_chart = (
    _base.mark_bar().encode(
        x=alt.X('Level1').sort(order="descending").title("Subpillar"),
        y=alt.Y('count').title("Number of Companies"),
        # color=alt.condition(_point, "stage", alt.value('lightgray')),
    )
    # .add_params(_point)
)

_rule = _base.mark_rule(strokeDash=[2, 2]).encode(
    y=alt.datum(2),
    color=alt.datum("england")
)

# This will cause error
_mo_chart = mo.ui.altair_chart((alt.layer(_chart, _rule)))
# _mo_chart = mo.ui.altair_chart((_chart))
_mo_chart
# alt.layer(_chart, _rule)
mscolnick commented 2 days ago

Thanks for sharing this repro. This is definitely a bug. marimo tries to add its own selection (automatically), so I think we need to handle the case when you've added your own selection.

Would you expect marimo to not add any selection or augment (if it can) with the selection params you have added? My guess it the former: if any selection is present, we don't add this for you.

ztawilvdl commented 2 days ago

@mscolnick I don't think I quite follow (I'm pretty new to Altair so pardon the ignorance). Why is the selection piece the issue when the chart is layered? I don't quite understand why that would affect the selection since everything works if it's not a layered chart.

Edit: And when doing a concat vs layer it works This will work

_base = alt.Chart(long_counts)

_brush = alt.selection_interval(encodings=['x'])
_point = alt.selection_point(encodings=['x'])
_chart = (
    _base.mark_bar().encode(
        x=alt.X('Level1').sort(order="descending").title("Subpillar"),
        y=alt.Y('count').title("Number of Companies"),
        color=alt.condition(_point, "stage", alt.value('lightgray')),
        tooltip=alt.Tooltip(["Level1", "count", "stage"])
    )
    .add_params(_point, _brush)
)

_rule = _base.mark_point(strokeDash=[2, 2]).encode(
    x=alt.X('Level1').sort(order="descending").title("Subpillar"),
    y=alt.Y('count').title("Number of Companies"),
    color=alt.datum("Early Venture")
).add_params(_point, _brush)

mo_chart = mo.ui.altair_chart((_chart | _rule))
# mo_chart = mo.ui.altair_chart((_chart))
mo_chart
mscolnick commented 2 days ago

mo.ui.altair_chart effectively adds selection as well, there is a chart_selection and field_selection param that you could try to toggle. I don't know the exact issue, but I imagine that our automatically added selection doesn't play nicely with your explicit selection.

ztawilvdl commented 2 days ago

Ooooh. I didn't realize that it added it. Weird that it works when it's not layered. When I remove the selection and let Marimo handle it, I see weird behaviour with the values updating

Freshly executed cell with the charts ✅ Image

Click middle bar, values update ✅ Image

Click right bar, values don't update ❌ Image

ztawilvdl commented 2 days ago

Going back to your question:

Would you expect marimo to not add any selection or augment (if it can) with the selection params you have added? My guess it the former: if any selection is present, we don't add this for you.

I don't know. I guess I don't like magic, so if I have a selection maybe you don't do any because I might not want it.

mscolnick commented 2 days ago

Yea, I think we likely just don't handle layers well at all. Given your examples, I can take a look and make sure we handle them properly (or warn why we cannot)

ztawilvdl commented 2 days ago

Sweet, this works I think:

test_counts = pd.DataFrame([
    {"Level1": "a", "count": 1, "stage": "france"},
    {"Level1": "b", "count": 2, "stage": "france"},
    {"Level1": "c", "count": 3, "stage": "england"}
])

_base = alt.Chart(test_counts)

_point = alt.selection_point(encodings=['x'])
_brush  = alt.selection_interval(encodings=['x'])
_chart = (
    _base.mark_bar().encode(
        x=alt.X('Level1').sort(order="descending").title("Subpillar"),
        y=alt.Y('count').title("Number of Companies"),
        color=alt.condition(_point, "stage", alt.value('lightgray')),
    )
    .add_params(_point, _brush)
)

_rule = _base.mark_rule(strokeDash=[2, 2]).encode(
    y=alt.datum(2),
    color=alt.datum("england")
)

# This will cause error
mo_chart1 = mo.ui.altair_chart(
    (alt.layer(_chart, _rule)),
    chart_selection=None,
)

mo_chart1
# alt.layer(_chart, _rule)