Skip to content

Make it easier to reference models' views #13351

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

Merged
merged 15 commits into from
Sep 15, 2023

Conversation

mattpap
Copy link
Contributor

@mattpap mattpap commented Aug 25, 2023

This makes Bokeh.index a ViewManager with a compatibility layer, so that Bokeh.index[id] still works. I also added access to the index to CustomJS callbacks via a new context (or cb_context) argument. Having such context is useful, because it allows users to avoid using globals, Bokeh specifically, which either may not be available (e.g. in ESM) or can be confusing when loading multiple instances of Bokeh on a page.

Due to issues with testing in my new setup, I had to resolve issue #12625.

TODO:

  • tests

addresses #13350
fixes #12625

@mattpap mattpap added this to the 3.3 milestone Aug 25, 2023
@codecov
Copy link

codecov bot commented Aug 25, 2023

Codecov Report

Merging #13351 (550eb5a) into branch-3.3 (277fd86) will increase coverage by 0.00%.
The diff coverage is 92.85%.

@@             Coverage Diff             @@
##           branch-3.3   #13351   +/-   ##
===========================================
  Coverage       92.42%   92.43%           
===========================================
  Files             316      316           
  Lines           20209    20218    +9     
===========================================
+ Hits            18679    18688    +9     
  Misses           1530     1530           

Copy link
Member

@bryevdv bryevdv left a comment

Choose a reason for hiding this comment

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

Really nice changes, just a few comments.

@bryevdv
Copy link
Member

bryevdv commented Aug 29, 2023

@mattpap just noting that #13350 was specifically about resetting from Python. I am not sure that's actually attained by this PR? (I think we'd need a new protocol event that could be triggered from Python to invoke the reset on the JS side)

Edit: I am not opposed to closing the issue with this PR, but I think we should explain the distinction and change in plans and why this is probably all that is needed, in the issue itself.

@bryevdv
Copy link
Member

bryevdv commented Aug 30, 2023

@mattpap as long it's now straightforward to expose reset in this way what you do think about exposing an explicit "request redraw" for users? Current users will try to do things like source.change.emit() to force a redraw (saw it again today)

@mattpap mattpap force-pushed the mattpap/13350_global_view_manager branch from 6cd48e4 to 861972a Compare September 14, 2023 07:51
@mattpap
Copy link
Contributor Author

mattpap commented Sep 14, 2023

in this way what you do think about exposing an explicit "request redraw" for users?

request_render() is the API for this purpose:

export default ({plot}, _target, _data, {index}) => {
    const plot_view = index.get_one(plot)
    plot_view.request_render()
}

@mattpap
Copy link
Contributor Author

mattpap commented Sep 14, 2023

just noting that #13350 was specifically about resetting from Python. I am not sure that's actually attained by this PR? (I think we'd need a new protocol event that could be triggered from Python to invoke the reset on the JS side)

Yeah, I so far overlooked the fact that we don't have any sensible programmatic way of running CustomJS (not attached to any tools or buttons or so).

@mattpap
Copy link
Contributor Author

mattpap commented Sep 14, 2023

I will handle supporting running CustomJS independently of UI controls in another PR.

@mattpap mattpap force-pushed the mattpap/13350_global_view_manager branch from 861972a to 550eb5a Compare September 15, 2023 06:23
@mattpap mattpap merged commit 3d1ecd1 into branch-3.3 Sep 15, 2023
@mattpap mattpap deleted the mattpap/13350_global_view_manager branch September 15, 2023 14:26
Chiemezuo pushed a commit to Chiemezuo/bokeh that referenced this pull request Aug 27, 2024
* Redesign Bokeh.index as a ViewManager

* Expose additional metadata in CustomJS

* Add interaction/js_callbacks/reset_plot example

* Update templates, export, etc. to new APIs

* More robust handling of chromedriver

Detects common variations in chromedriver executable's naming
(e.g. chromedriver-binary, chromium.chromedriver) and adds
BOKEH_CHROMEDRIVER_PATH setting.

* Update unit tests

* Add typings for webdriver.chrome.service

* Rename meta to context

* Finalize implementation of index proxy

* Allow to dispose a ViewManager

* Allow to construct a Document with roots

* Add unit tests

* Rename Document.{_add_root->_add_roots}

* Improve CustomJS.code's docstring

* Improve Settings.chromedriver_path's docstring
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

export_png doesn't find chrome webdriver because it was installed as chromedriver-binary
2 participants