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

Update visible UI from annotator events #123

Merged
merged 14 commits into from
Aug 31, 2024
Merged

Update visible UI from annotator events #123

merged 14 commits into from
Aug 31, 2024

Conversation

droumis
Copy link
Member

@droumis droumis commented Jul 10, 2024

Fixes #124

Previously, the PanelWidgets' Visibility UI did not update when new annotations were added/removed/changed. This PR attempts to synchronize the visibility UI with annotator events: create, delete, and update (edit).

I've also changed how tabulator on_edit triggers events.. previously any change was triggering both field and region update events, and now it's either one xor the other.

I've also moved the legend color circles to the left of the visibility UI instead of the right, given that if you added more than 4 groupby field types, then a scrollbar appears and obscures the legend color circles on the right. The aesthetics of this can still be much improved, but at least it's not interfering now.

TODO:

  • Fix coloring. Make annotator glyph coloring to respect the new persistent approach - (a new groupby field value gets assigned a color and it will stay that color regardless if you delete all the annotations of that field value and then create more). Instead of the persistent approach (saved for a future discussion/PR), I'm now just following what the annotation glyphs are doing.. essentially annotation groups (if color is unspecified by the user) will change color based on sorted order.
  • add tests
Code
 
 
from holonote.annotate import Annotator 
from holonote.app.tabulator import AnnotatorTable 
from holonote.app import PanelWidgets  
import panel as pn; pn.extension('tabulator')
import holoviews as hv; hv.extension('bokeh')
from holonote.annotate.connector import SQLiteDB

annotator = Annotator({"height": float, "width": float}, fields=["type"],
                      connector=SQLiteDB(filename=':memory:'))
annotator.groupby = "type"
annotator_widgets = pn.Column(PanelWidgets(annotator), AnnotatorTable(annotator))

pn.Column(annotator_widgets, annotator * hv.Image([], ['width', 'height'])).servable()

Basic Functionality working pretty well:

GMT20240710-004725_Clip_Demetris.Roumis.s.Clip.07_09_2024.mp4

Copy link

codspeed-hq bot commented Jul 10, 2024

CodSpeed Performance Report

Merging #123 will not alter performance

Comparing visibility-gui (5d86ba3) with main (57aa6b3)

Summary

✅ 6 untouched benchmarks

Copy link

codecov bot commented Jul 10, 2024

Codecov Report

Attention: Patch coverage is 84.37500% with 25 lines in your changes missing coverage. Please review.

Project coverage is 86.74%. Comparing base (57aa6b3) to head (5d86ba3).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
holonote/app/panel.py 68.25% 20 Missing ⚠️
holonote/app/tabulator.py 0.00% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #123      +/-   ##
==========================================
+ Coverage   86.34%   86.74%   +0.40%     
==========================================
  Files          26       26              
  Lines        2775     2920     +145     
==========================================
+ Hits         2396     2533     +137     
- Misses        379      387       +8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@droumis
Copy link
Member Author

droumis commented Jul 10, 2024

I'm stuck. The coloring seems pretty broken/out of sync with the actual annotations, and I don't think I'll be able to fix it myself. Any help would be very welcome! I was hoping it would be easy to just grab and apply a colormap from the annotations and use that for the visibility widget, but I don't see a way.

I'm happy to forget about imposing the persistent colormap idea (added in this PR for the visible UI), but even then, I don't understand how to sync with the colors of the annotations on the plot per groupby field category/type.

It's a bit complicated because the user may or may not provide a colormap as an hv.dim. If they do, this hv.dim may be partial, as users can continue to create/edit additional categories not originally specified by the hv.dim.

example of user-specified colors
 
 
from holonote.annotate import Annotator 
from holonote.app.tabulator import AnnotatorTable 
from holonote.app import PanelWidgets  
import panel as pn; pn.extension('tabulator')
import holoviews as hv; hv.extension('bokeh')
from holonote.annotate.connector import SQLiteDB

 
annotator = Annotator({"height": float, "width": float}, fields=["type"],
                      connector=SQLiteDB(filename=':memory:'))
color_dim = hv.dim("type").categorize(
    categories={"A": "blue", "B": "red", "C": "green"}, default="grey"
)  

annotator.style.color = color_dim 

annotator.groupby = "type"
annotator_widgets = pn.Column(PanelWidgets(annotator), AnnotatorTable(annotator))

pn.Column(annotator_widgets, annotator * hv.Image([], ['width', 'height'])).servable()

Remaining problem of the colors of the annotations not being colored the same way as the visibility UI legend:

GMT20240710-004941_Clip_Demetris.Roumis.s.Clip.07_09_2024.mp4

@ahuang11
Copy link
Contributor

To clarify the issue, when you delete the box annotation, the colors get out of sync and you want to grab the color of the new box?

if color not in used_colors:
return color
# If all default colors are used, cycle through again
return _default_color[len(self.colormap) % len(_default_color)]
Copy link
Contributor

Choose a reason for hiding this comment

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

Wondering if you could use hv.Cycle here.

@droumis
Copy link
Member Author

droumis commented Jul 10, 2024

To clarify the issue, when you delete the box annotation, the colors get out of sync and you want to grab the color of the new box?

Yes, except the colors were never really technically in sync; it just so happens that, in some conditions (e.g. if no color mapping is specified by the user), they appear to be in sync when they really are not. And this lack of syncing becomes apparent when you delete all of a certain group of annotations, or when a new group is created that gets sorted between existing groups. The way the visible UI is generating colors is just different than how colors are generated for the annotation boxes. I haven't figured out yet how to sync them.

@ahuang11
Copy link
Contributor

ahuang11 commented Jul 10, 2024

A way to sync I think is by taking inspiration from how HoloViews curve and scatter plots have the same colors, by both having their own instances of hv.Cycle: https://github.com/holoviz/holoviews/blob/80bc964ff51e360d34fc81c0d33f128dab136005/holoviews/plotting/bokeh/__init__.py#L163-L172

@droumis
Copy link
Member Author

droumis commented Jul 10, 2024

A way to sync I think is by taking inspiration from how HoloViews curve and scatter plots have the same colors, by both having their own instances of hv.Cycle

hmmm, I'm not sure I understand how to implement this in the context of a potentially partially user-specified colormap

@ahuang11
Copy link
Contributor

ahuang11 commented Jul 10, 2024

I believe hv.Cycle(user_specified_cmap), e.g. hv.Cycle("RdBu") works but I could be missing an important aspect.

@ahuang11
Copy link
Contributor

Something like this?

import holoviews as hv
import matplotlib

hv.extension("bokeh")

# get list from colormaps
cmap = matplotlib.colormaps["RdBu_r"]
colors = [matplotlib.colors.rgb2hex(cmap(i)) for i in range(2)]
cmap_a = hv.Cycle(colors)
cmap_b = hv.Cycle(colors)

hv.Curve([1, 2, 3]).opts(color=cmap_a) * hv.Curve([3, 2, 1]).opts(color=cmap_b)

Comment on lines 145 to 146
self.visible_widget.options = sorted([*old_options, new_option])
self.visible_widget.value = [*old_values, new_option]
Copy link
Member

Choose a reason for hiding this comment

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

Always better to update all parameters at once.

Suggested change
self.visible_widget.options = sorted([*old_options, new_option])
self.visible_widget.value = [*old_values, new_option]
self.visible_widget.param.update(
options=sorted([*old_options, new_option]),
value=[*old_values, new_option]
)

The same applies below.

Copy link
Member Author

Choose a reason for hiding this comment

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

thanks for the tip, I committed this change in this particular 'create' event type. The other place where two parameters are updated is under the 'update' event, but I will keep that separate because if a group (e.g. 'A') had been set to not be visible, and then an annotation that is the last of it's kind is updated (e.g. from 'B' to 'A'), then I would want the group ('A') to remain not visible, even though we now have to update the 'options' parameter. That is why I am keeping them separate for that particular event type.

@droumis
Copy link
Member Author

droumis commented Jul 19, 2024

Condition: color is not defined by user, groupby is applied, no annotations at initialization

Checking if colors stay synced with the default colormap when groups become resorted (from update, creation, or deletion)

Code
from holonote.annotate import Annotator 
from holonote.app.tabulator import AnnotatorTable 
from holonote.app import PanelWidgets  
import panel as pn; pn.extension('tabulator')
import holoviews as hv; hv.extension('bokeh')
from holonote.annotate.connector import SQLiteDB
 
annotator = Annotator({"x": float,}, fields=["type"],
                      connector=SQLiteDB(filename=':memory:'))

annotator.groupby = "type"
annotator_widgets = pn.Column(PanelWidgets(annotator), AnnotatorTable(annotator))

pn.Column(annotator_widgets, annotator * hv.Curve([]).opts(xlim=(-1,3))).servable()
GMT20240719-230730_Clip_Color.is.not.defined.by.user.groupby.is.applied.no.annotations.at.initialization.mp4

@droumis
Copy link
Member Author

droumis commented Jul 19, 2024

Condition: color is defined by user with hv.dim, groupby is applied, annotations present at initialization

Checking same as above, but also if new annotation types that are not specified in the hv.dim dict get assigned to the 'default' color grey. Also checking if sole annotations (only in its type) get assigned to same color after deletion and then recreation.

Code
import pandas as pd
from holonote.annotate import Annotator 
from holonote.app.tabulator import AnnotatorTable 
from holonote.app import PanelWidgets  
import panel as pn; pn.extension('tabulator')
import holoviews as hv; hv.extension('bokeh')
from holonote.annotate.connector import SQLiteDB

data = {
    'type': ['A', 'B', 'C'],
    'start': range(3),
    'end': [i + .8 for i in range(3)]
}

annotations_df = pd.DataFrame(data)
 
annotator = Annotator({"x": float,}, fields=["type"],
                      connector=SQLiteDB(filename=':memory:'))

annotator.define_annotations(annotations_df, x=("start", "end"))

color_dim = hv.dim("type").categorize(
    categories={"A": "purple", "B": "orange", "C": "green"}, default="grey"
)

annotator.style.color = color_dim 

annotator.groupby = "type"
annotator_widgets = pn.Column(PanelWidgets(annotator), AnnotatorTable(annotator))

pn.Column(annotator_widgets, annotator * hv.Curve([]).opts(xlim=(-1,3))).servable()
GMT20240719-231455_Clip_color.is.defined.by.user.with.hvdim.groupby.is.applied.no.annotations.at.initializ.mp4

@droumis
Copy link
Member Author

droumis commented Jul 19, 2024

Condition: color is defined by user with hv.dim, groupby is applied, BUT only SOME annotations are present at initialization which match with the entries of the color dim

Checking specifically that the color remains consistent when I create>delete>create an annotation for that group which did not have an annotation at initialization, but whose color is specified by the user regardless.

Code
import pandas as pd
from holonote.annotate import Annotator 
from holonote.app.tabulator import AnnotatorTable 
from holonote.app import PanelWidgets  
import panel as pn; pn.extension('tabulator')
import holoviews as hv; hv.extension('bokeh')
from holonote.annotate.connector import SQLiteDB

data = {
    'type': ['A', 'B'],
    'start': range(2),
    'end': [i + .8 for i in range(2)]
}

annotations_df = pd.DataFrame(data)
 
annotator = Annotator({"x": float,}, fields=["type"],
                      connector=SQLiteDB(filename=':memory:'))

annotator.define_annotations(annotations_df, x=("start", "end"))

color_dim = hv.dim("type").categorize(
    categories={"A": "purple", "B": "orange", "C": "green"}, default="grey"
)

annotator.style.color = color_dim 

annotator.groupby = "type" 
annotator_widgets = pn.Column(PanelWidgets(annotator), AnnotatorTable(annotator))

pn.Column(annotator_widgets, annotator * hv.Curve([]).opts(xlim=(-1,3))).servable()
GMT20240719-232111_Clip_Partial.init.annotations.dim.color.specified.mp4

@droumis
Copy link
Member Author

droumis commented Jul 19, 2024

Condition: Color is defined by user as a string, groupby applied, no annotations at initialization

Checking for any signs of chaos.. should be a straightforward condition.

Code
## COLOR IS STRING, GROUPBY, NO ANNOTATIONS INIT
import pandas as pd
from holonote.annotate import Annotator 
from holonote.app.tabulator import AnnotatorTable 
from holonote.app import PanelWidgets  
import panel as pn; pn.extension('tabulator')
import holoviews as hv; hv.extension('bokeh')
from holonote.annotate.connector import SQLiteDB

data = {
    'type': ['A', 'B', 'C'],
    'start': range(3),
    'end': [i + .8 for i in range(3)]
}

annotations_df = pd.DataFrame(data)
 
annotator = Annotator({"x": float,}, fields=["type"],
                      connector=SQLiteDB(filename=':memory:'))

annotator.define_annotations(annotations_df, x=("start", "end"))

annotator.style.color = 'red' 

annotator.groupby = "type"
annotator_widgets = pn.Column(PanelWidgets(annotator), AnnotatorTable(annotator))

pn.Column(annotator_widgets, annotator * hv.Curve([]).opts(xlim=(-1,3))).servable()
GMT20240719-232653_Clip_Demetris.Roumis.s.Clip.07_19_2024.mp4

@droumis
Copy link
Member Author

droumis commented Jul 22, 2024

Condition: color is defined by user with hv.dim, groupby is applied, annotations are not present at initialization

Checking that the widgets can be initialized without data and then correctly applied the specified colormap

I also now made it so that an entry requires data to be included in the visibility widget.. preventing issues with annotator.visible state.

Code
from holonote.annotate import Annotator 
from holonote.app.tabulator import AnnotatorTable 
from holonote.app import PanelWidgets  
import panel as pn; pn.extension('tabulator')
import holoviews as hv; hv.extension('bokeh')
from holonote.annotate.connector import SQLiteDB
 
annotator = Annotator({"x": float,}, fields=["type"],
                      connector=SQLiteDB(filename=':memory:'))

color_dim = hv.dim("type").categorize(
    categories={"A": "purple", "B": "orange", "C": "green"}, default="grey"
)

annotator.style.color = color_dim 

annotator.groupby = "type"
annotator_widgets = pn.Column(PanelWidgets(annotator), AnnotatorTable(annotator))

pn.Column(annotator_widgets, annotator * hv.Curve([]).opts(xlim=(-1,3))).servable()
vizui.mp4

@droumis droumis requested a review from hoxbro July 23, 2024 15:12
@droumis droumis marked this pull request as ready for review July 23, 2024 15:12
elif isinstance(style.color, str):
self.colormap = dict(zip(options, [style.color] * len(options)))
elif isinstance(style.color, hv.dim):
self.colormap = self.annotator.style.color.ops[0]["kwargs"]["categories"]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
self.colormap = self.annotator.style.color.ops[0]["kwargs"]["categories"]
self.colormap = dict(self.annotator.style.color.ops[0]["kwargs"]["categories"])

Copy link
Member

Choose a reason for hiding this comment

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

Alternatively could you do something like:

colormap = dict(zip(options, style.color.apply(hv.Dataset(options, [self.annotator.groupby]))))

Copy link
Member Author

Choose a reason for hiding this comment

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

I think @hoxbro had an initial impression that this suggestion might be too heavy for this particular use case, but I'll let him clarify/expound in his review

@droumis
Copy link
Member Author

droumis commented Aug 5, 2024

@hoxbro , is this ready to merge?

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.

Looks good.

Can you check if the changes look in Google Chrome, Firefox, and Safari with respect to the stylesheet change?

@droumis droumis self-assigned this Aug 6, 2024
@droumis
Copy link
Member Author

droumis commented Aug 31, 2024

@hoxbro. Good call. Firefox was a bit misaligned and Safari doesn't display the circles at all. Firefox was an easy fix, but unfortunately, it seems like the circles were never appearing in Safari.

From what I can tell, styling a select element isn't easily done in Safari. I tried to placing the circles inline with the option element as SVGs so as to avoid using ':after' psedo class. I also started trying to created a select wrapper class, but I couldn't get it to work with Safari at all.

I propose that we merge this PR, knowing the colored circles are not yet showing in Safari, and then work towards replacing the Multiselect component with something like a selectable tabulator. This would also make it easier to add additional group-level batch actions: recoloring, deleting, renaming, etc because we could put additional buttons in the additional columns.

Before

Chrome:
image

Firefox:
image

Safari:
image

After most recent commit:

Chrome:
image

Firefox:
image

Safari:
image

holonote/tests/conftest.py Outdated Show resolved Hide resolved
@hoxbro hoxbro enabled auto-merge (squash) August 31, 2024 06:41
@hoxbro hoxbro disabled auto-merge August 31, 2024 06:41
@hoxbro hoxbro enabled auto-merge (squash) August 31, 2024 06:41
@hoxbro hoxbro merged commit eb9dbf4 into main Aug 31, 2024
13 checks passed
@hoxbro hoxbro deleted the visibility-gui branch August 31, 2024 06:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Visible widget doesn't update with new 'groupby field' values
4 participants