Adding Echarts events

[JackDapid]

Adding Echarts events support to Echarts modal #2147

• create initial JS events - done
• min. Echarts test app - done
• next step: sending back params to python
• next step: improved tests (from app to autom. test as in tests folder)
• next step: some doc

[JackDapid]

@MarcSkovMadsen & @philippjfr I just pushed a first 'fully' functional version of echarts events - creating JS events as well as getting the subscribed events back as python params works - missing points:

• agreement of the event-config convention, e.g. 'function ( ) { SCRIPT }' together with query
• more thinking and agreement of event filtering as different echarts series return useful param info in different ways
• understanding why pytest fail - I started that process
• proposing some doc
• implementing a pytest for events

[JackDapid]

I added custom select and filters:
click': {'query': 'series.tree', 'select': 'data'}, # Tests for Echarts event with custom select
'click': {'query': 'series.tree', 'filters': ['children']}, # Tests for Echarts event with custom filters
'click': {'query': 'series.tree', 'select': 'data', 'filters': ['children']}, # Tests for Echarts event with custom select and filters

[JackDapid]

not sure I understand the signature pytest errors, some help is appreciated.

One problem with event readonly?:
FAILED panel/tests/pane/test_base.py::test_pane_clone[ECharts] - TypeError: Read-only parameter 'event' cannot be modified under investigation.
can be fixed by removing the readonly=True, from:
https://github.com/JackDapid/panel/blob/84aa237a8c8936d45db4c0a9590f937512850061/panel/pane/echarts.py#L23
but is that wanted?

[MarcSkovMadsen]

Hi @JackDapid

My argument for having event readonly is that if you show the parameter in a notebook or app then it is nice that the TextInput is disabled. I think @philippjfr is the one who can answer if readonly=True should be removed during testing or something else should be done.

[JackDapid]

Thanks @MarcSkovMadsen und Danke @philippjfr

yes, that was also my reason and happy to either leave it out or propose a modification of the test_pane_clone to leave out read-onlies. Signature issue was caused by (stupidly) moving the wrong echarts example to the example folder - fixed now.

For a more general discussion on how I (and we @gluoNNet, adding @kjp) could contribute to Panel I would like to point you to a PR (Press Release and not Pull Request :-)) http://www.caakz.com/aviation-administration-of-kazakhstan-launches-digital-regulator-sunflower/ and video, where we showcase some of our heavy use of Holoviews, Panel. ....
https://youtu.be/tUaAj3u_vvA?t=952
If intersting for you I would love a discussion with both of you @philippjfr and @MarcSkovMadsen how we can contribute to Panel?, awesome-panel?, ...?, maybe showcase some use-cases .... We were 'dragged' into Panel by awesome-panel, use our own modified react template, ... so only through this PR (Pull Request again ;-) ) realised the @philippjfr Holoviews, Datashader to @MarcSkovMadsen Panel-awesome connection :-)

[codecov]

Codecov Report

Merging #2174 (7091321) into main (817363f) will decrease coverage by 9.60%.
The diff coverage is 85.71%.

@@            Coverage Diff             @@
##             main    #2174      +/-   ##
==========================================
- Coverage   83.17%   73.58%   -9.60%     
==========================================
  Files         247      247              
  Lines       36337    36401      +64     
==========================================
- Hits        30224    26785    -3439     
- Misses       6113     9616    +3503     

Flag	Coverage Δ
ui-tests	?
unitexamples-tests	73.58% <85.71%> (+0.06%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
panel/models/echarts.py	77.77% <63.63%> (-10.46%)	⬇️
panel/pane/echarts.py	76.28% <79.16%> (+1.28%)	⬆️
panel/tests/pane/test_echart.py	100.00% <100.00%> (+66.66%)	⬆️

... and 39 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[philippjfr]

I'd really like to provide some detailed feedback here because the functionality is definitely great but I'm missing a bit of context. A lot of that could be answered by some documentation that describes the scope and implementation of the event system.

So for now a couple of general points:

Events vs Parameters

I would not require coupling events to parameters, we should leverage Bokeh events more and provide ways for users to subscribe to those using an on_event method. This is a more general task as, in theory, a lot of Panel components could allow subscribing to such events. The new ReactiveHTML component does this by providing the on_event method which allows users to subscribe to arbitrary DOM events on a specific DOM node.

By using a real event system rather than abusing parameter/properties for this we won't have to merge all the events into a single event property and eliminate the need to name specific events.

A real specification

I'd love to see a full specification of the event config proposed here. I see things like query, select and filters in the event config but don't quite know what they mean so if you could write that up that would be very helpful. Also instead of a handler for JS callbacks I'd prefer adding a separate js_on_event method which allows registering custom callbacks, which we can then internally wrap with bokeh CustomJS calls so they can reference other Bokeh models.

Happy to expand on any of this, but some references that might help to see what I'm talking about are the Python implementation of on_event (https://github.com/holoviz/panel/blob/master/panel/reactive.py#L1362), the definition of the custom Bokeh DOMEvent (https://github.com/holoviz/panel/blob/master/panel/models/reactive_html.py#L144), the DOM event listener setup (https://github.com/holoviz/panel/blob/master/panel/models/reactive_html.ts#L333) and the dispatching of the Bokeh events (https://github.com/holoviz/panel/blob/master/panel/models/reactive_html.ts#L183)

[MarcSkovMadsen]

Hi @JackDapid

I'm just blown away by the presentation. You have really put together a strong application based on HoloViz and Panel.

I am also really interested in connecting and discussing.

My guess is that you have a lot of Ideas on where Panel should be improved, you already have some improvements that could improve Panel in general and showcasing and explaining some of your work would be truly amazing.

I'm including @nghenzi here who created the React Template. @nghenzi checkout the videos in the links above to see how your Template has been taken to the next level.

[philippjfr]

Agreed, absolutely incredible work on that application. Thanks for sharing and as @MarcSkovMadsen indicated we'd be very grateful for your feedback.

[JackDapid]

@philippjfr and @MarcSkovMadsen - not forgotten, just delivery deadline this Friday and will come back to it either later this week or during the weekend - agree to almost all written looking forward to next steps

[JackDapid]

@philippjfr here some context for the events. The original idea was 'just' to get some client side JS event handling for echarts, so for example clicking on a node in the echarts - while discussing with @MarcSkovMadsen it turned out that we could easily add both server side and client side event support for echarts, which we than implemented - not aiming at a general Panel events support. But open for ideas, clearly.
So currently possible to subscribe to events (selection possible by query, filter and select settings - which for sure need improved doc and also some examples) and all that for echarts only. Does that help with the context or did I misunderstood the question?

[philippjfr]

I'm reviving this, but do have one question. Are the query and filter fields in the event config really needed? Sending these events is not expensive so I'd rather have a simpler configuration and make the whole event available in Python.

[philippjfr]

@JackDapid Will merge shortly but if you have suggestions or issues with the approach please file an issue.