Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add as_popup #101

Merged
merged 25 commits into from
May 7, 2024
Merged

Add as_popup #101

merged 25 commits into from
May 7, 2024

Conversation

ahuang11
Copy link
Contributor

@ahuang11 ahuang11 commented Apr 23, 2024

Upon completing region to highlight, a popup appears with the widgets
Clicking anywhere will popup widgets to allow editing / committing

Screen.Recording.2024-04-24.at.8.48.58.AM.mov
import panel as pn
import numpy as np
import pandas as pd
import hvplot.pandas
import numpy as np
import holoviews as hv

from holonote.annotate import SQLiteDB, Annotator
from holonote.app import PanelWidgets

pn.extension()

curve = pd.read_parquet("assets/example.parquet").hvplot(x="TIME")
connector = SQLiteDB(table_name="test_app")
fields = ["Stoppage", "Reason", "Category"]
annotator = Annotator({"TIME": np.datetime64}, fields=fields, connector=connector)
annotator_element = annotator * curve

fields_values = {
    "Stoppage": ["Yes", "No"],
    "Category": ["Mechanical", "Electrical", "Process", "Other"],
}

w = PanelWidgets(annotator, field_values=fields_values, as_popup=True)
pn.serve(annotator_element)

@ahuang11 ahuang11 requested a review from hoxbro April 23, 2024 19:11
@hoxbro
Copy link
Member

hoxbro commented Apr 23, 2024

This does not seem ready to be reviewed.

@ahuang11
Copy link
Contributor Author

ahuang11 commented Apr 23, 2024

Forgot to mention at the end, I think I wanted feedback before adding tests, i.e. what you envision for two popups.

@hoxbro
Copy link
Member

hoxbro commented Apr 23, 2024

Can you run the pre-commit?

Copy link

codspeed-hq bot commented Apr 23, 2024

CodSpeed Performance Report

Merging #101 will not alter performance

Comparing add_as_popup (350ba19) with main (1df707a)

Summary

✅ 6 untouched benchmarks

@ahuang11
Copy link
Contributor Author

Maybe we could use right click to pop up a menu:
image

import numpy as np

from bokeh.io import show
from bokeh.models import (ActionItem, BoxSelectTool, CheckableItem,
                          CustomJS, DividerItem, Menu)
from bokeh.palettes import Spectral11
from bokeh.plotting import figure

N = 1000
x = np.random.random(size=N) * 100
y = np.random.random(size=N) * 100
radii = np.random.random(size=N) * 1.5
colors = np.random.choice(Spectral11, size=N)

plot = figure(active_scroll="wheel_zoom")
cr = plot.circle(x, y, radius=radii, fill_color=colors, fill_alpha=0.6, line_color=None)

box_select = BoxSelectTool(persistent=True)
plot.add_tools(box_select)

delete_selected = CustomJS(
    args=dict(renderer=cr),
    code="""
export default ({renderer}) => {
    const {entries} = Bokeh.require("core/util/object")
    const {enumerate} = Bokeh.require("core/util/iterator")
    const {data, selected} = renderer.data_source
    const indices = new Set(selected.indices)
    const new_data = {}
    for (const [name, column] of entries(data)) {
        new_data[name] = column.filter((value, i) => !indices.has(i))
    }
    renderer.data_source.data = new_data
    renderer.data_source.selected.indices = [] // TODO bug in ds update
}
    """,
)

change_color = CustomJS(
    args=dict(renderer=cr),
    code="""
export default ({renderer}, _menu, {item}) => {
    const {data, selected} = renderer.data_source
    const indices = new Set(selected.indices)
    const selected_color = item.label
    const fill_color = [...data["fill_color"]]
    for (const i of indices) {
        fill_color[i] = selected_color
    }
    renderer.data_source.data = {...data, fill_color}
}
    """,
)

change_continuous = CustomJS(
    args=dict(box_select=box_select),
    code="""
export default ({box_select}, _obj, {item}) => {
    const {continuous} = box_select
    box_select.continuous = item.checked = !continuous
}
    """,
)

invert_selection = CustomJS(
    args=dict(renderer=cr),
    code="""
export default ({renderer, overlay}) => {
    renderer.selection_manager.invert()
}
    """,
)

clear_selection = CustomJS(
    args=dict(renderer=cr, overlay=box_select.overlay),
    code="""
export default ({renderer, overlay}) => {
    overlay.visible = false
    renderer.data_source.selected.indices = []
}
    """,
)

menu = Menu(
    items=[
        ActionItem(
            label="Count",
            shortcut="Alt+C",
            disabled=True,
            action=CustomJS(code="""console.log("not implemented")"""),
        ),
        ActionItem(
            label="Delete",
            shortcut="Alt+Shift+D",
            icon="delete",
            action=delete_selected,
        ),
        DividerItem(),
        ActionItem(
            label="Choose color",
            menu=Menu(
                stylesheets=[
                    "\n".join([f".color-{color.removeprefix('#')} {{ background-color: {color}; }}" for color in Spectral11]),
                    ".bk-label { font-family: monospace; }",
                ],
                items=[
                    ActionItem(
                        label=color,
                        icon=f".color-{color.removeprefix('#')}",
                        action=change_color,
                    ) for color in Spectral11
                ],
            ),
        ),
        DividerItem(),
        CheckableItem(
            label="Continuous selection",
            checked=box_select.continuous,
            action=change_continuous,
        ),
        DividerItem(),
        ActionItem(
            icon="invert_selection",
            label="Invert selection",
            action=invert_selection,
        ),
        ActionItem(
            icon="clear_selection",
            label="Clear selection",
            shortcut="Esc",
            action=clear_selection,
        ),
    ],
)
box_select.overlay.context_menu = menu
show(plot)

@ahuang11
Copy link
Contributor Author

Added functionality where double click clears, click anywhere shows the widgets, click on select shows the widgets + delete.

Screen.Recording.2024-04-23.at.3.50.52.PM.mov

@ahuang11
Copy link
Contributor Author

I dropped some HoloNote original behavior so that it doesn't require re-clicking the same glyph upon changing from "+" to "-" and vice versa.

Screen.Recording.2024-04-24.at.7.49.14.AM.mov

@ahuang11
Copy link
Contributor Author

Okay this is ready for review when you have the time.

@hoxbro
Copy link
Member

hoxbro commented Apr 30, 2024

How do these changes affect annotation when you are not using the pop-up?

@ahuang11
Copy link
Contributor Author

it works the same way if I'm not mistaken (must have holoviz/holoviews#6207 alongside):

Screen.Recording.2024-04-30.at.6.48.39.AM.mov

you can also have it both:

import panel as pn
import numpy as np
import pandas as pd
import hvplot.pandas
import numpy as np
import holoviews as hv

from holonote.annotate import SQLiteDB, Annotator
from holonote.app import PanelWidgets

pn.extension()

curve = pd.read_parquet("assets/example.parquet").hvplot(x="TIME")
connector = SQLiteDB(table_name="test_app")
fields = ["Stoppage", "Reason", "Category"]
annotator = Annotator({"TIME": np.datetime64}, fields=fields, connector=connector)
annotator_element = annotator * curve

fields_values = {
    "Stoppage": ["Yes", "No"],
    "Category": ["Mechanical", "Electrical", "Process", "Other"],
}

w = PanelWidgets(annotator, field_values=fields_values, as_popup=True)
pn.Row(w, annotator_element).show()
Screen.Recording.2024-04-30.at.7.02.33.AM.mov

@hoxbro
Copy link
Member

hoxbro commented Apr 30, 2024

Same thing if you want to edit?

And what if you don't use the Panel widgets at all?

@ahuang11
Copy link
Contributor Author

And what if you don't use the Panel widgets at all?

Can you clarify what you mean?

@hoxbro
Copy link
Member

hoxbro commented Apr 30, 2024

Does this work if you have don't use PanelWidgets at all? The tutorial notebooks 1 to 3.

@ahuang11
Copy link
Contributor Author

ahuang11 commented Apr 30, 2024

Okay, the edit tool works by untangling the tap + double tap:

  • tap popup when selection_enabled
  • double tap toggle popup; if already visible clears it, else shows it.
Screen.Recording.2024-04-30.at.8.38.11.AM.mov

The tutorials also work AFAIK

@ahuang11
Copy link
Contributor Author

ahuang11 commented May 6, 2024

When you get the chance, would appreciate another review.

@hoxbro
Copy link
Member

hoxbro commented May 6, 2024

I'm playing around with this PR as we speak.

hoxbro added 2 commits May 6, 2024 16:49
Copy link
Member

@hoxbro hoxbro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, this PR is in a good state.

I noticed that after you double-click, you will get two pop-ups, making for a confusing experience.

screenrecord-2024-05-06_17.01.01.mp4

Another thing is sometimes I see the pop-up "collapsing". What I mean by this is the inside of the pop-up becomes invisible, and the pop-up is empty with only the x before it disappears completely.

I'm also not a very big fan of the comment # Rerun to avoid Models must be owned by only a single document. I couldn't recreate it in Jupyter, but it could be a VSCode-only problem. I think we should be able to remove this problem (not sure where exactly).

holonote/annotate/display.py Outdated Show resolved Hide resolved
@ahuang11
Copy link
Contributor Author

ahuang11 commented May 6, 2024

I noticed that after you double-click, you will get two pop-ups, making for a confusing experience.

Yes I agree that the two popups is annoying, but I have no idea how to prevent it (they're the same panel). Maybe @philippjfr has an idea?

What I mean by this is the inside of the pop-up becomes invisible, and the pop-up is empty with only the x before it disappears completely.

I think this could be fixed by changing the order of things

I'm also not a very big fan of the comment # Rerun to avoid Models must be owned by only a single document.` I couldn't recreate it in Jupyter, but it could be a VSCode-only problem.

Not sure how to handle too.

@hoxbro
Copy link
Member

hoxbro commented May 6, 2024

Yes I agree that the two popups is annoying

Yeah, I just saw your comment in HoloViews.

Not sure how to handle too.

The first step is you describe where you are seeing the problem 🙂

@philippjfr
Copy link
Member

Yes I agree that the two popups is annoying, but I have no idea how to prevent it (they're the same panel). Maybe @philippjfr has an idea?

Unless you have two separate streams attached I don't think it should be possible to have multiple. If you do have multiple streams you probably just need to toggle visibility on one when you open the other.

@ahuang11
Copy link
Contributor Author

ahuang11 commented May 6, 2024

The first step is you describe where you are seeing the problem 🙂

The notebook if you drop that line and run from the start.
image

RuntimeError: Models must be owned by only a single document, BoxAnnotation(id='3f621686-6abd-4ffa-a39e-5b00173e79b2', ...) is already in a doc
:DynamicMap   []
   :Overlay
      .Curve.I      :Curve   [TIME]   (SPEED)
      .Curve.II     :Curve   [TIME]   (y)
      .NdOverlay.I  :NdOverlay   [Element]
         :VSpans   [start[TIME],end[TIME]]   (Stoppage,Reason,Category,__selected__)
      .NdOverlay.II :NdOverlay   [Element]
         :VSpan   [x,y]

@ahuang11
Copy link
Contributor Author

ahuang11 commented May 6, 2024

If you do have multiple streams you probably just need to toggle visibility on one when you open the other.

Yes, we have a double tap + box select stream. They both use the same Panel layout, but potentially we could create a dict to track them?

examples/tutorial/04_Make_an_App.ipynb Outdated Show resolved Hide resolved
examples/tutorial/04_Make_an_App.ipynb Outdated Show resolved Hide resolved
@ahuang11
Copy link
Contributor Author

ahuang11 commented May 7, 2024

layouts are now tracked and if one popup is visible, the others are made invisible

Screen.Recording.2024-05-06.at.8.46.18.PM.mov

Also, was applying this and it works decently:

from holonote.annotate import Annotator
from holonote.app import PanelWidgets
import xarray as xr
import panel as pn
import holoviews as hv

hv.extension("bokeh")

ds = xr.tutorial.open_dataset("air_temperature")


def plot_image(time):
    image = hv.Image(ds.sel(time=time), ["lon", "lat"], ["air"]).opts(
        cmap="RdBu_r", title=time, colorbar=True, clim=(220, 340), width=500, height=300
    )
    return image


def plot_timeseries_by_select(indices):
    if indices:
        row = annotator.df.loc[indices[0]]
        lon1 = row["start[lon]"]
        lon2 = row["end[lon]"]
        lat1 = row["start[lat]"]
        lat2 = row["end[lat]"]
        ds_sel = ds.sel(lon=slice(lon1, lon2), lat=slice(lat2, lat1)).mean(
            ["lat", "lon"]
        )
        time_series.object = hv.Curve(ds_sel["air"]).opts(
            title=f"Time Series {lon1:.2f} {lat1:.2f} {lon2:.2f} {lat2:.2f}", width=500
        )


def plot_timeseries_by_stream(bounds):
    if not bounds:
        lon1, lat1, lon2, lat2 = ds.lon.min(), ds.lat.min(), ds.lon.max(), ds.lat.max()
        ds_sel = ds
    else:
        lon1, lat1, lon2, lat2 = bounds
        ds_sel = ds.sel(lon=slice(lon1, lon2), lat=slice(lat2, lat1))
    time_series.object = hv.Curve(ds_sel["air"].mean(["lat", "lon"])).opts(
        title=f"Time Series {lon1:.2f} {lat1:.2f} {lon2:.2f} {lat2:.2f}", width=500
    )


times = ds.time.dt.strftime("%Y-%m-%d %H:%M").values.tolist()

# start annotation
annotator = Annotator({"lon": float, "lat": float}, fields=["Description", "Time", "Z"])
annotator_widgets = PanelWidgets(
    annotator, field_values={"Time": times, "Z": 100}, as_popup=True
)

# make image dependent on the selected time
time_input = annotator_widgets.fields_widgets[1]
time_input.value = times[0]
image = hv.DynamicMap(pn.bind(plot_image, time_input))

time_series = pn.pane.HoloViews()

# update plot time when a new box is SELECTED
pn.bind(plot_timeseries_by_select, annotator.param.selected_indices, watch=True)

# update plot time when a new box is CREATED
display = annotator.get_display("lat", "lon")
box_stream = display._edit_streams[0]  # to make public later
box_stream.source = image
pn.bind(plot_timeseries_by_stream, box_stream.param.bounds, watch=True)

# layout
pn.Row(annotator_widgets, pn.Column(annotator * image, time_series)).show()
Screen.Recording.2024-05-06.at.8.44.52.PM.mov

I think we need to make _edit_streams public, and also maybe have a param to disable resetting/refreshing/clearing the widgets value.

@hoxbro
Copy link
Member

hoxbro commented May 7, 2024

I think we need to make _edit_streams public

I have no objections to this. I think we should do this in another PR.

and also maybe have a param to disable resetting/refreshing/clearing the widgets value.

That is also OK with me and can be done in this PR.

@ahuang11
Copy link
Contributor Author

ahuang11 commented May 7, 2024

Merging if no objections~

@hoxbro hoxbro added enhancement New feature or request and removed needs-review labels May 7, 2024
@hoxbro hoxbro merged commit d0def9d into main May 7, 2024
17 checks passed
@hoxbro hoxbro deleted the add_as_popup branch May 7, 2024 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants