-
-
Notifications
You must be signed in to change notification settings - Fork 402
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
Support async callbacks for popup #6390
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6390 +/- ##
==========================================
- Coverage 88.49% 87.73% -0.77%
==========================================
Files 323 323
Lines 68180 68478 +298
==========================================
- Hits 60338 60080 -258
- Misses 7842 8398 +556 ☔ View full report in Codecov by Sentry. |
async_execute(self._process_selection_event) | ||
self._skipped_partial_event = False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a safe change?
self._skipped_partial_event
will be set to False immediately and not after the method has been called.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the purpose of skipped_partial_event, i.e. how should I test it?
I think maybe I can move self._skipped_partial_event inside the process_selection_event then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure, just wanted to flag that you are making a potential change.
I'm not sure you can move it inside either, as other calls to the method that previously did not reset it will now do it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well tests seem to point to lasso select failing even though it seems to be working on my end
https://github.com/holoviz/holoviews/actions/runs/11056395453/job/30717855703?pr=6390
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok I think wrapping it in its own helper function should keep old behavior
Maybe @philippjfr has more insights on |
It works in my notebook, but I think when I serve it, I get this error with the async executor changes: ERROR:tornado.application:Exception in callback functools.partial(<bound method IOLoop._discard_future_result of <tornado.platform.asyncio.AsyncIOMainLoop object at 0x1069b5db0>>, <Task finished name='Task-92' coro=<PopupMixin._process_selection_event() done, defined at [/Users/ahuang/repos/holoviews/holoviews/plotting/bokeh/callbacks.py:676](https://file+.vscode-resource.vscode-cdn.net/Users/ahuang/repos/holoviews/holoviews/plotting/bokeh/callbacks.py:676)> exception=RuntimeError('_pending_writes should be non-None when we have a document lock, and we should have the lock when the document changes')>)
Traceback (most recent call last):
File "/Users/ahuang/miniconda3/envs/holoviews/lib/python3.10/site-packages/tornado/ioloop.py", line 738, in _run_callback
ret = callback()
File "/Users/ahuang/miniconda3/envs/holoviews/lib/python3.10/site-packages/tornado/ioloop.py", line 762, in _discard_future_result
future.result()
File "/Users/ahuang/repos/holoviews/holoviews/plotting/bokeh/callbacks.py", line 728, in _process_selection_event
self._panel.visible = True
File "/Users/ahuang/miniconda3/envs/holoviews/lib/python3.10/site-packages/bokeh/core/has_props.py", line 336, in __setattr__
return super().__setattr__(name, value)
File "/Users/ahuang/miniconda3/envs/holoviews/lib/python3.10/site-packages/bokeh/core/property/descriptors.py", line 332, in __set__
self._set(obj, old, value, setter=setter)
File "/Users/ahuang/miniconda3/envs/holoviews/lib/python3.10/site-packages/bokeh/core/property/descriptors.py", line 620, in _set
self._trigger(obj, old, value, hint=hint, setter=setter)
File "/Users/ahuang/miniconda3/envs/holoviews/lib/python3.10/site-packages/bokeh/core/property/descriptors.py", line 698, in _trigger
obj.trigger(self.name, old, value, hint, setter)
File "/Users/ahuang/miniconda3/envs/holoviews/lib/python3.10/site-packages/bokeh/model/model.py", line 571, in trigger
super().trigger(descriptor.name, old, new, hint=hint, setter=setter)
File "/Users/ahuang/miniconda3/envs/holoviews/lib/python3.10/site-packages/bokeh/util/callback_manager.py", line 186, in trigger
self.document.callbacks.notify_change(cast(Model, self), attr, old, new, hint, setter, invoke)
File "/Users/ahuang/miniconda3/envs/holoviews/lib/python3.10/site-packages/bokeh/document/callbacks.py", line 251, in notify_change
self.trigger_on_change(event)
File "/Users/ahuang/miniconda3/envs/holoviews/lib/python3.10/site-packages/bokeh/document/callbacks.py", line 423, in trigger_on_change
invoke_with_curdoc(doc, invoke_callbacks)
File "/Users/ahuang/miniconda3/envs/holoviews/lib/python3.10/site-packages/bokeh/document/callbacks.py", line 453, in invoke_with_curdoc
return f()
File "/Users/ahuang/miniconda3/envs/holoviews/lib/python3.10/site-packages/bokeh/document/callbacks.py", line 422, in invoke_callbacks
cb(event)
File "/Users/ahuang/miniconda3/envs/holoviews/lib/python3.10/site-packages/bokeh/document/callbacks.py", line 278, in <lambda>
self._change_callbacks[receiver] = lambda event: event.dispatch(receiver)
File "/Users/ahuang/miniconda3/envs/holoviews/lib/python3.10/site-packages/bokeh/document/events.py", line 352, in dispatch
super().dispatch(receiver)
File "/Users/ahuang/miniconda3/envs/holoviews/lib/python3.10/site-packages/bokeh/document/events.py", line 218, in dispatch
cast(DocumentPatchedMixin, receiver)._document_patched(self)
File "/Users/ahuang/miniconda3/envs/holoviews/lib/python3.10/site-packages/bokeh/server/session.py", line 244, in _document_patched
raise RuntimeError("_pending_writes should be non-None when we have a document lock, and we should have the lock when the document changes")
RuntimeError: _pending_writes should be non-None when we have a document lock, and we should have the lock when the document changes |
Ok I'm stuck; can't seem to figure a way to get around that document lock; works in notebook, but not server |
Closes #6388
Screen.Recording.2024-09-26.at.8.41.32.AM.mov