holoviz / holonote

Tools to create, edit, and persist annotated regions for HoloViews
https://holonote.holoviz.org
BSD 3-Clause "New" or "Revised" License
23 stars 2 forks source link

Add support for custom tabulator options #126

Closed droumis closed 4 months ago

droumis commented 4 months ago

Fixes #125

Use tabulator_kwargs and refer to the options https://panel.holoviz.org/reference/widgets/Tabulator.html

TODO:

For instance:

AnnotatorTable(annotator, tabulator_kwargs=dict(pagination='local', page_size=7))
Code ```python import panel as pn import holoviews as hv hv.extension('bokeh') import pandas as pd from holonote.annotate import Annotator from holonote.annotate.connector import SQLiteDB from holonote.app import PanelWidgets, AnnotatorTable data = { 'description': ['T0', 'T1', 'T2'] * 10, 'start': range(30), 'end': [i + .8 for i in range(30)] } annotations_df = pd.DataFrame(data) curve_data = { 'time': range(30), 'val': [1, 3, 2] * 10, } curve_df = pd.DataFrame(curve_data) plot = hv.Curve(curve_df, 'time', 'val') annotator = Annotator({'time': float}, fields=["description"], connector=SQLiteDB(filename=':memory:') ) annotator.define_annotations(annotations_df, time=("start", "end")) annotator.groupby = "description" annotator.visible = ["T1"] annotator_widgets = pn.Column( PanelWidgets(annotator), AnnotatorTable( annotator, tabulator_kwargs=dict( pagination='local', page_size=7, ) ), width=400 ) # Display the widgets and plot pn.Row(annotator_widgets, annotator * plot.opts(responsive=True)).servable() ```
codspeed-hq[bot] commented 4 months ago

CodSpeed Performance Report

Merging #126 will not alter performance

Comparing tabulator_kwargs (ffe1ce7) with main (ceb60de)

Summary

✅ 6 untouched benchmarks

codecov[bot] commented 4 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 86.34%. Comparing base (ceb60de) to head (ffe1ce7).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #126 +/- ## ======================================= Coverage 86.33% 86.34% ======================================= Files 26 26 Lines 2774 2775 +1 ======================================= + Hits 2395 2396 +1 Misses 379 379 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

droumis commented 4 months ago

Note to self, I was thinking about adding input validation warnings for dict items like this:

class _TabulatorOpts(param.Dict):
    def __init__(self, default={}, doc="kwargs to pass to tabulator", allow_refs=True):
        super().__init__(default=default, doc=doc, allow_refs=allow_refs)

    def _validate(self, val) -> None:
        super()._validate(val)
        warn_opts = ("show_index", "selectable", "buttons")
        for k in val:
            if k in warn_opts:
                warn(f"`{k}` is set directly and should not be passed through `tabulator_opts`")

But then the doc entry doesn't appear in the docstring, so I think I'll leave it out and just let the validation be a side effect of passing in a duplicate kwarg of one that is set directly

ahuang11 commented 4 months ago

You could create your own param type with validation, e.g.

tabulator_kwargs = param.TabulatorKwargs()

As an example, Panel does it for a few like Margin. https://github.com/holoviz/panel/blob/9f412a0cb6a3093c0a0f34489ae24db85bea60c6/panel/_param.py#L51

droumis commented 4 months ago

I think that's pretty much what I proposed in the comment above, no? I think I'll avoid for now because I prefer the docs to show up, but thanks. How does this PR look currently?

ahuang11 commented 4 months ago

I think the PR looks fine; I'm just not sure if allow_refs=True is necessary or not since I don't have any experience with it so I was hoping for Philipp to review :P Are you able to confirm it doesn't work when allow_refs=False?

I think that's pretty much what I proposed in the comment above,

Oops yeah, I didn't recognize it since I'm used to seeing inheriting from param.Parameter

the docs to show up

I'm a bit confused about this one.

from param import Parameter, Parameterized

class Aspect(Parameter):
    """
    A Parameter type to validate aspect ratios. Supports numeric values
    and auto.
    """

    def __init__(self, default=None, allow_None=True, **params):
        super().__init__(default=default, allow_None=allow_None, **params)
        self._validate(default)

    def _validate(self, val):
        self._validate_value(val, self.allow_None)

    def _validate_value(self, val, allow_None):
        ...

class Test(Parameterized):

    tabulator_aspect = Aspect(doc="Just testing")

Test?

results in

Init signature: Test(*, tabulator_aspect, name)
Docstring:     
Parameters of 'Test'
====================

Parameters changed from their default values are marked in red.
Soft bound values are marked in cyan.
C/V= Constant/Variable, RO/RW = ReadOnly/ReadWrite, AN=Allow None

Name              Value   Type     Mode  

tabulator_aspect   None  Aspect  V RW AN 

Parameter docstrings:
=====================

tabulator_aspect: Just testing
Type:           ParameterizedMetaclass
Subclasses:     
philippjfr commented 4 months ago

There's no reason not to enable allow_refs but it isn't strictly needed. Tabulator allows refs so that's all that's necessary, enabling it here would make it possible to bind references to these objects which is also nice.