Skip to content
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

Update panel to use bokeh build #626

Merged
merged 19 commits into from Oct 23, 2019
Merged

Update panel to use bokeh build #626

merged 19 commits into from Oct 23, 2019

Conversation

mattpap
Copy link
Collaborator

@mattpap mattpap commented Aug 28, 2019

This is a preview based on a WIP PR bokeh/bokeh#9198, where the ad-hoc extensions compiler was replaced with a new, more robust one. One can build panel with bokeh build panel (assuming cwd is the repository) or with bokeh.ext.build("panel") (which is used in setup.py). The changes in this PR reflect how the extension should (almost) ideally look, barring any backwards compatibility issues. Almost, because e.g. require_components() could be managed by bokeh, or __module__ fields could be inserted automatically, similarly to __name__. The extension layout isn't set in stone, so if there are any reservations regarding the current approach, then things can still change.

Example output:

~/repo/panel $ bokeh build panel/
Working directory: /home/mateusz/repo/panel/panel
Using different version of bokeh, rebuilding from scratch.
Running npm install.
added 32 packages from 50 contributors and audited 272 packages in 15.457s
found 0 vulnerabilities

Using /home/mateusz/repo/panel/panel/tsconfig.json
Compiling TypeScript (13 files)
Linking modules
Output written to /home/mateusz/repo/panel/panel/dist
All done.

@philippjfr
Copy link
Member

@philippjfr philippjfr commented Sep 9, 2019

Thanks so much for this. Will play with this now and hopefully merge it today or tomorrow. To be clear, which version of bokeh will contain the build refactors required to make this work? Will this be in a 1.3.5 or 1.4.0 release?

@mattpap
Copy link
Collaborator Author

@mattpap mattpap commented Sep 9, 2019

This is based on a WIP PR (bokeh/bokeh#9198), so hold on with a merge until that is merged. I submitted this for a preview and to gather some initial impressions on whether the overall work around bokeh build is going in the right direction. Feedback is welcome, both here and in bokeh's PR. As I said in the top-level comment, things aren't set in stone yet, so this is a good time to point out any deficiencies or inconveniences before we release this work (bokeh 1.4).

@philippjfr
Copy link
Member

@philippjfr philippjfr commented Sep 9, 2019

I'm running into some issues with this. The first is a weird git issue I don't understand, when I checked out your branch it never created the bokeh.ext.json for some reason so I had to create manually. I also don't see it in https://github.com/mattpap/panel/tree/build but it does show up in the diff?!

My second issue was that the build step in setup.py doesn't seem to work:

def _build_paneljs():
    from bokeh.ext import build
    print("Building custom models:")
    build()

TypeError: build() missing 1 required positional argument: 'base_dir'

I made a suggestion on how to fix that. With that fix everything seems to build correctly but in the neither in the notebook or the server can I get the models to actually be initialized and am seeing this error in the console:

connection.js:1 Error: Model 'panel.models.widgets.VideoStream' does not exist. This could be due to a widget or a custom model not being registered before first usage.
    at Object.o.Models (base.js:1)
    at Function.e._instantiate_object (document.js:1)
    at Function.e._instantiate_references_json (document.js:1)
    at Function.e.from_json (document.js:1)

setup.py Outdated
def _build_paneljs():
from bokeh.ext import build
print("Building custom models:")
build()
Copy link
Member

@philippjfr philippjfr Sep 9, 2019

Choose a reason for hiding this comment

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

Suggested change
build()
basepath = os.path.split(__file__)[0]
build(basepath)

@philippjfr
Copy link
Member

@philippjfr philippjfr commented Sep 9, 2019

@mattpap Also let me know if there's anything else related to this in bokeh or here that is currently blocked by me.

@mattpap
Copy link
Collaborator Author

@mattpap mattpap commented Sep 10, 2019

TypeError: build() missing 1 required positional argument: 'base_dir'

Eh, I didn't eventually test this, as I couldn't build the conda package due to having an insufficient amount of spare RAM.

Model 'panel.models.widgets.VideoStream' does not exist. This could be due to a widget or a custom model not being registered before first usage.

This should work. My immediate concern is that the version of bokehjs you are running isn't the one from the bokeh's PR. Assuming bokehjs was built correctly, in the case of the notebook, I would suggest to run it like this:

BOKEH_RESOURCES=server-dev jupyter notebook

and in another terminal:

bokeh static

or bokeh server, because static seems broken right now (I have a fix locally).

In any case, I will update this PR later today.

@mattpap
Copy link
Collaborator Author

@mattpap mattpap commented Sep 10, 2019

On a side note, I also intend to drop require_components() and make bokehjs' build aware of dependencies loaded from external sources like unpkg, etc., but I'm not sure if this will happen in PR bokeh/bokeh#9198 or a followup one.

@philippjfr
Copy link
Member

@philippjfr philippjfr commented Sep 10, 2019

Separately, is there any chance the bokeh PR could be made backward compatible? Currently I'm getting:

error TS18003: No inputs were found in config file 'tsconfig.json'. Specified 'include' paths were '["./**/*.ts"]' and 'exclude' paths were '["./dist"]'.

@xavArtley
Copy link
Collaborator

@xavArtley xavArtley commented Sep 14, 2019

hello @mattpap

I tried the PR but I encounter same errors as @philippjfr

As you suggest I start jupyter lab using
set BOKEH_RESOURCES=server-dev& jupyter lab
and python -m bokeh serve in \bokeh\bokehjs renaming build as static directory
but I still get
image

If I try to use bokeh output_notebook command bokehjs loading seeams fine
image

@xavArtley
Copy link
Collaborator

@xavArtley xavArtley commented Sep 14, 2019

Running python setup.py develop from panel directory is not building the extensions
So I changed the fix of @philippjfr in :

basepath = os.path.split(__file__)[0]
build(os.path.join(basepath,'panel'))

However the build seems to stop after npm install. and nothing was created

running develop
Building custom models:
Working directory: C:\Data\PythonWorkspace\modules\panel\panel
Using different version of bokeh, rebuilding from scratch.
Running npm install.
running egg_info

Windows issue?

@xavArtley
Copy link
Collaborator

@xavArtley xavArtley commented Sep 15, 2019

Ok so it's a windows issue, to make compilation works I had to change in build.ts
https://github.com/bokeh/bokeh/blob/c71cc1ce8c177608eab8deaec976c719739fe623/bokehjs/src/compiler/build.ts#L12
and add in options {shell: true}
Concerning the setup.py just replaceing build() by build('panel') makes the trick
now the log is :

C:\Data\PythonWorkspace\pyenv\lib\site-packages\setuptools\dist.py:475: UserWarning: Normalizing '0.7.0a10.post13+g6f6e6a0-dirty' to '0.7.0a10.post13+g6f6e6a0.dirty'
  normalized_version,
running develop
Building custom models:
Working directory: C:\Data\PythonWorkspace\modules\panel\panel
Using different version of bokeh, rebuilding from scratch.
Running npm install.
updated 1 package and audited 272 packages in 2.156s
found 0 vulnerabilities

Using C:\Data\PythonWorkspace\modules\panel\panel\tsconfig.json
Compiling TypeScript (13 files)
Linking modules
Output written to C:\Data\PythonWorkspace\modules\panel\panel\dist
All done.

However in notebook context I have always the same error:

Uncaught Error: Model 'panel.models.vtk.VTKPlot' does not exist. This could be due to a widget or a custom model not being registered before first usage.
    at Object.o.Models (eval at append_javascript (outputarea.js:762), <anonymous>:309:144)

but the widget works in server (using show() method on the panel object)

@gagelarsen
Copy link

@gagelarsen gagelarsen commented Sep 18, 2019

Any update here? I am also getting the error mentioned above (#626 (comment)) by @philippjfr concerning the tsconfig.json file

@xavArtley
Copy link
Collaborator

@xavArtley xavArtley commented Sep 18, 2019

I have this error too, I have to downdate bokeh to 1.3.2

@philippjfr
Copy link
Member

@philippjfr philippjfr commented Sep 18, 2019

To be clear, which error? None of this is has been released in bokeh.

@philippjfr
Copy link
Member

@philippjfr philippjfr commented Sep 18, 2019

And I'm routinely using bokeh 1.3.4 with panel 0.6.2 and panel master.

@xavArtley
Copy link
Collaborator

@xavArtley xavArtley commented Sep 18, 2019

error TS18003: No inputs were found in config file 'tsconfig.json'. Specified 'include' paths were '["./**/*.ts"]' and 'exclude' paths were '["./dist"]'.

After trying the build PR I had to downdate bokeh to an official one

@philippjfr
Copy link
Member

@philippjfr philippjfr commented Sep 18, 2019

Ah right, yeah, but bokeh 1.3.4 should work. I also had some issues downgrading again though, ended up checking out bokeh master and deleting all dist and build directories and then rebuilding.

@xavArtley
Copy link
Collaborator

@xavArtley xavArtley commented Sep 18, 2019

Yes indeed I tapped pip install "bokeh<1.3.4" and I finished with version 1.3.2 (no version 1.3.3 on pypi apparently)

@gagelarsen
Copy link

@gagelarsen gagelarsen commented Sep 18, 2019

I am using bokeh 1.3.5 - it has some features we require for a project we are working on.

@philippjfr
Copy link
Member

@philippjfr philippjfr commented Sep 18, 2019

There is no bokeh 1.3.5 yet unless you mean a dev release. I'd also expect that to work.

@gagelarsen
Copy link

@gagelarsen gagelarsen commented Sep 18, 2019

i am using bokeh 1.3.5dev2 on the there dev channel on conda

@mattpap
Copy link
Collaborator Author

@mattpap mattpap commented Oct 21, 2019

@philippjfr, can you push your changes, even if incomplete or broken. I would like to get this work finished.

@philippjfr
Copy link
Member

@philippjfr philippjfr commented Oct 22, 2019

@mattpap Sorry about that, just saw this. I'm working on this now.

@mattpap
Copy link
Collaborator Author

@mattpap mattpap commented Oct 22, 2019

Sorry about that, just saw this. I'm working on this now.

No problem. If don't have time for this, I can finish this, just I don't want to reinvent what we already did.

@philippjfr philippjfr force-pushed the build branch 2 times, most recently from 06d0025 to 174ce73 Compare Oct 22, 2019
@philippjfr philippjfr force-pushed the build branch 2 times, most recently from c6ffe49 to b8b4db8 Compare Oct 23, 2019
@philippjfr
Copy link
Member

@philippjfr philippjfr commented Oct 23, 2019

@mattpap I've got this passing by doing a simple python setup.py develop for now. Using a conda build required a bit too much reshuffling which I'll do at some later point.

@mattpap mattpap closed this Oct 23, 2019
@mattpap mattpap reopened this Oct 23, 2019
@philippjfr philippjfr merged commit 5c452a6 into holoviz:master Oct 23, 2019
1 of 2 checks passed
@mattpap mattpap deleted the build branch Oct 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants