Implementation of user-defined models #2800
Conversation
This is required to make sure bokehjs doesn't depend on require() provided by the parent environment. It's also helpful for debugging.
This is needed to support meaningful module identifiers. By default, monotonically increasing counter is used by browserify, to reduce bundle size. However, we need to identify modules after compilation, so we need more meaningful IDs. New IDs are based on module path, just system-independent and with prefix and extension removed.
This allows users to extend bokehjs' functionality with custom behaviour, e.g. add new tools, actions/callbacks, etc. To make a custom model, the user has to provide __implementation__ field which is a string with CoffeeScript source code.
Conflicts: bokeh/embed.py bokehjs/package.json
|
Hi Matt, Thanks for working on this, I'm looking forward to it. It would be great to be able to easily use pyscript , like in #2799, instead of writing JS for this feature, as an option. Is this feasible? |
Conflicts: bokeh/embed.py
|
@mattpap this is really awesome! If I understand correctly the goal it'd also make very easy to write "bokeh extentions" to add new sets of widgets based on other web UI libs out there.. |
|
@datnamer, pyscript is just a different syntax from what is supported in this PR, so it's not a concern of this PR. But yes, I think it's feasible to support it as soon as we figure out the semantics of this feature. It actually may be even simpler to implement than the current in-browser compilation of coffeescript. On a side note, bokeh is not only Python, but also R, Scala, etc., so whatever "nice" syntax we support, we have to also have the common default for other languages/platforms/runtime environments. |
@fpliger, this might be the case, but it depends how deeply such developments have to integrate with bokehjs. For now I'm only considering simple extensions like this |
|
@mattpap Gotcha, makes sense. +1 on my end to extensive custom UI and widget support :) |
Conflicts: bokeh/models/callbacks.py
Conflicts: bokeh/enums.py
|
@damianavila, both merged. I'm still working on making |
This way there is little chance we will interfere with the parent environment. It would be best if we didn't have to set any globals at all, but this may not be possible.
|
Some tests are still failing @mattpap Also, this a big PR... I have made a general overview and looks OK at overall, but next time we should be more strict with ourselves and not merge big PRs like this one just before the release without an actual detailed review... Please @mattpap don't take this against you... I certainly know this PR has low probabilities to introduce bugs, essentially because you write it, but we should make our review process better... ping @bokeh/core, sorry to be annoying with these kind of things, but I think this is pretty important... BTW, testing the examples now ... I will push the fix for the building machinery to get this new js and css into the CDN and merge this one after the test are fixed...
Yes, I agree... |
@damianavila, the fun part is that I ran |
|
Just now I noticed this: |
@damianavila, I'm actually against merging this PR as-is, because, on the contrary, it has very high probability of introducing (possibly subtle) bugs. I'm just following the general desire to have this merged for 0.10.
That's not annoying. The sole fact that you are forced to ping the team multiple times is annoying. |
…url generation in resources.
| 'css_files' : ['%sbokehjs/static/css/bokeh%s.css' % (root_url, _min)], | ||
|
|
||
| def mk_url(comp, tp): | ||
| _comp = '-' + comp if comp else '' |
There was a problem hiding this comment.
I am also worried about this line... because you get something like:
...bokehjs/static/js/-bokeh.min.js
There was a problem hiding this comment.
Yes, that's a bug. Originally comp was one of '' or 'widgets'. I moved the bokeh- prefix, make this more readable. The fun part is that server examples work. Why? Because, server doesn't use resources. I'm not sure really what uses this.
No. The same thing as with server resource URLs. |
Everyone is fully committed or more likely over-committed at the moment. While I can see how this might be annoying from your viewpoint, this sort of comment is easy to read as an indictment against individuals, and is not very productive (in my opinion). Regarding this PR: it's late in the release schedule, let's hold off. |
OK, let's wait for this one... retrying the examples they do not always work for me (sometimes clicks do not raise the alert) and with more time, making bokeh-widgets optional could be done here (or in a subsequent PR) and all the task enumerated here completed... thanks for the discussion and let continue with the review of this one after the release. |
|
@mattpap please create a |
|
@bryevdv, I think this PR should be left as-is and merged just after 0.10 release, and of course followed up by a few PRs with finishing touches. This way the team will have a chance to test this properly over, hopefully, sufficiently long period of time. |
|
Merged into master? That implies there are no |
|
Yes, I should have said that explicitly, I think the next release should be 0.11, especially if we want to keep up with the 1.0 plan, you've proposed. |
OK, going forward with this one, I will merge this now... |
Implementation of user-defined models
This will include split of bokehjs as well at some point. This PR is intentionally based on
master. To test this, try outglyphs/customexample. Click on a circle and a popup window should appear with a message including fill color of the circle.