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

Pre-compile custom models and code fragments #4946

Merged
merged 28 commits into from Sep 7, 2016
Merged

Pre-compile custom models and code fragments #4946

merged 28 commits into from Sep 7, 2016

Conversation

@mattpap
Copy link
Contributor

@mattpap mattpap commented Aug 11, 2016

This removes run-time bokeh-compiler.js bundle. Custom models are now precompiled, dependencies extracted (to reasonable extent) and verified. JavaScript is parsed and syntax analyzed as well. There is support for importing Less/CSS stylesheets. node.js is a required dependency for custom models, where CoffeeScript is the default and dependency parsing necessary. In CustomJS/FuncTickFormatter JavaScript is still the default and dependencies are not resolved by default. This way one can create CustomJS without node.js. See custom.py and transform_jitter*.py under models/. This all needs some further polishing and perhaps additional changes to package build files, so that bokehjs/build/js/{compile.js,modules.json} are included. I squashed all changes to a single commit (original history was garbage).

fixes #4879, #4886

@mattpap mattpap mentioned this pull request Aug 11, 2016
2 of 2 tasks complete
@@ -27,14 +27,14 @@
plot.add_layout(LinearAxis(), 'below')
plot.add_layout(LinearAxis(), 'left')

customjs = CustomJS(args=dict(source=source), lang="coffeescript", code="""
customjs = CustomJS(args=dict(source=source), code=CoffeeScript("""

This comment has been minimized.

@bryevdv

bryevdv Aug 11, 2016
Member

This is a breaking change. People are definitely making custom extensions at this point, and @birdsarah and I have both been pushing the idea pretty solidly. I think this needs a deprecation warning. Sholud be pretty simple, add an __init__ to CustomJS that strips the lang kwarg and converts/wraps code accordingly?

This comment has been minimized.

@mattpap

mattpap Aug 12, 2016
Author Contributor

Actually this example changes further more to CustomJS.from_coffeescript(args=..., code="""..."""), so I'm not sure how we want to deprecate things here.

@@ -162,7 +162,7 @@ then the complete Python class might look like:
class Custom(LayoutDOM):
__implementation__ = open("custom.coffee").read()
__implementation__ = "custom.coffee"

This comment has been minimized.

@bryevdv

bryevdv Aug 11, 2016
Member

also a breaking change, is there a way to have a deprecation cycle?

This comment has been minimized.

@mattpap

mattpap Aug 12, 2016
Author Contributor

__implementation__ accepts a string or an instance implementing Implementation interface. Accepting string is a convenience measure and, by default, nothing has changed here. A string is considered CoffeeScript source code. However, if a string is a one-liner which ends in .coffee, .js, .css or .less, then it will be interpreted as a path (relative to file where a model is defined). This new convenience feature shouldn't conflict with any earlier code.

@@ -158,9 +158,6 @@ def enumeration(*values, **kwargs):
#: Specify an aggregation type for different charts
Aggregation = enumeration("sum", "mean", "count", "nunique", "median", "min", "max")

#: Specify the language used in a CustomJS callback
ScriptingLanguage = enumeration("javascript", "coffeescript")

This comment has been minimized.

@bryevdv

bryevdv Aug 11, 2016
Member

prefer to leave in as a deprecated module attr for a short while

This comment has been minimized.

@mattpap

mattpap Aug 12, 2016
Author Contributor

My policy is to get rid of old code in the first place, to make sure we use new APIs everywhere and then deprecate. I will do it later.

@@ -56,17 +64,10 @@ def from_py_func(cls, func):
""")

code = String(default="", help="""
A snippet of JavaScript/CoffeeScript code to execute in the browser. The
A snippet of JavaScript code to execute in the browser. The

This comment has been minimized.

@bryevdv

bryevdv Aug 11, 2016
Member

Would be appropriate to mention the JS string can be made with CoffeeScript class from etc.

This comment has been minimized.

@mattpap

mattpap Aug 22, 2016
Author Contributor

This was addressed, just not visible in the short diff.

@@ -0,0 +1,274 @@
from __future__ import absolute_import

This comment has been minimized.

@bryevdv

bryevdv Aug 11, 2016
Member

I have a possibly odd but definitely strong aversion to having module names be gerunds (i.e "xyz-ing") What about util.compile or util.compiler or util.jsbuild ?

This comment has been minimized.

@mattpap

mattpap Aug 12, 2016
Author Contributor

compile conflicts with Python's compile(), so perhaps compiler. jsbuild is wrong as well, because you can do more than compiling JavaScript in this module.

throw new Error("Cannot find Bokeh. You have to load it prior to loading plugins.");
}
})
"""

This comment has been minimized.

@bryevdv

bryevdv Aug 11, 2016
Member

Is there a way to coordinate this automatically? Maybe setup.py could copy to server/static/js and this could read from there?

This comment has been minimized.

@mattpap

mattpap Aug 12, 2016
Author Contributor

Of course possible in at least a few ways, but for now wasn't significant enough to do. Also, the code was slowly converging from a different implementation to what plugins use. This outcome wasn't that obvious during early work.

@bryevdv
Copy link
Member

@bryevdv bryevdv commented Aug 11, 2016

@mattpap missing new npm dep?

    Error: Cannot find module 'acorn' from '/home/travis/build/bokeh/bokeh/bokehjs/src/js'
      at /home/travis/build/bokeh/bokeh/bokehjs/node_modules/resolve/lib/async.js:46:17
      at process (/home/travis/build/bokeh/bokeh/bokehjs/node_modules/resolve/lib/async.js:173:43)
      at ondir (/home/travis/build/bokeh/bokeh/bokehjs/node_modules/resolve/lib/async.js:188:17)
      at load (/home/travis/build/bokeh/bokeh/bokehjs/node_modules/resolve/lib/async.js:69:43)
      at onex (/home/travis/build/bokeh/bokeh/bokehjs/node_modules/resolve/lib/async.js:92:31)
      at /home/travis/build/bokeh/bokeh/bokehjs/node_modules/resolve/lib/async.js:22:47
      at FSReqWrap.oncomplete (fs.js:82:15)
@mattpap
Copy link
Contributor Author

@mattpap mattpap commented Aug 12, 2016

missing new npm dep?

The same with less. npm is fun.

@mattpap
Copy link
Contributor Author

@mattpap mattpap commented Aug 15, 2016

@bryevdv, to get things moving, I need conda package for node.js > 5.0 (this includes npm > 3.0). EDIT: actually this can be either 4.4.7+ (lts) or 6.0+ (cutting edge). 5.x is EOL and unsupported anymore.

@bryevdv
Copy link
Member

@bryevdv bryevdv commented Aug 16, 2016

@mattpap are you asking me to build such a package? I'm not sure offhand, do we know who built the existing one?

@mattpap
Copy link
Contributor Author

@mattpap mattpap commented Aug 16, 2016

are you asking me to build such a package? I'm not sure offhand, do we know who built the existing one?

I was asking for feedback, because I don't know my self how this was done before and who did it. Maybe @damianavila knows?

@bryevdv
Copy link
Member

@bryevdv bryevdv commented Aug 16, 2016

@mattpap we could try starting with the conda-forge recipe:

https://github.com/conda-forge/nodejs-feedstock/blob/master/recipe

and then if that works, uploading the result to the bokeh channel on anaconda.org

@bryevdv
Copy link
Member

@bryevdv bryevdv commented Aug 16, 2016

@mattpap the following modified meta.yaml (with the existing build.sh from the link above) seems to be working on OSX. If you build for linux we can upload to the bokeh channel:

{% set version = "5.0.0" %}

package:
  name: nodejs
  version: {{ version }}

source:
  fn: node-v{{ version }}.tar.gz
  url: https://nodejs.org/dist/v{{ version }}/node-v{{ version }}.tar.gz

build:
  number: 0
  #skip: True  # [not py27]

requirements:
  build:
    # node's build scripts find and use MSVC 2015, regardless of conda's setting.
    - vs2015_runtime  # [win]
    - python          2.7.*
    - gcc             # [linux]
  run:
    - vs2015_runtime   # [win]
    - libgcc           # [linux]

test:
  commands:
    - node -h
    - node -v
    - npm version
    - npm install -h
    - npm config get prefix -g
    - test $(echo "console.log(1 + 3)" | node) == 4  # [not win]

about:
  home: https://nodejs.org/
  license: MIT
  summary: a platform for easily building fast, scalable network applications

extra:
  recipe-maintainers:
    - msarahan
    - pelson
@bryevdv
Copy link
Member

@bryevdv bryevdv commented Aug 16, 2016

OSX version is up on anaconda.org with these commands:

$ anaconda upload /Users/bryan/anaconda/conda-bld/osx-64/nodejs-5.0.0-0.tar.bz2 -u bokeh
Using Anaconda Cloud api site https://api.anaconda.org
detecting package type ...
conda
extracting package attributes for upload ...
done

Uploading file bokeh/nodejs/5.0.0/osx-64/nodejs-5.0.0-0.tar.bz2 ... 
 uploaded 6940 of 6940Kb: 100.00% ETA: 0.0 minutes


Upload(s) Complete

Package located at:
https://anaconda.org/bokeh/nodejs

bryan@0199-bryanv  /tmp $ conda install -c bokeh nodejs
Using Anaconda Cloud api site https://api.anaconda.org
Fetching package metadata ...........
Solving package specifications: ..........

Package plan for installation in environment /Users/bryan/anaconda:

The following packages will be downloaded:

    package                    |            build
    ---------------------------|-----------------
    nodejs-5.0.0               |                0         6.8 MB  bokeh

The following packages will be UPDATED:

    nodejs: 4.1.0-1 javascript --> 5.0.0-0 bokeh

Proceed ([y]/n)? 
@bryevdv
Copy link
Member

@bryevdv bryevdv commented Aug 16, 2016

Although I get an error when I npm install

$ npm install
Error: Cannot find module './lib'
    at Function.Module._resolveFilename (module.js:337:15)
    at Function.Module._load (module.js:287:25)
    at Module.require (module.js:366:17)
    at require (module.js:385:17)
    at Object.<anonymous> (/Users/bryan/anaconda/lib/node_modules/npm/node_modules/request/node_modules/hawk/index.js:1:80)
    at Module._compile (module.js:425:26)
    at Object.Module._extensions..js (module.js:432:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:311:12)
    at Module.require (module.js:366:17)
    at require (module.js:385:17)
npm ERR! Darwin 15.6.0
npm ERR! argv "/Users/bryan/anaconda/bin/node" "/Users/bryan/anaconda/bin/npm" "install"
npm ERR! node v5.0.0
npm ERR! npm  v3.3.6
npm ERR! code MODULE_NOT_FOUND

npm ERR! Cannot find module './lib'
npm ERR! 
npm ERR! If you need help, you may report this error at:
npm ERR!     <https://github.com/npm/npm/issues>

@msarahan is this somethign you can help with?

@bryevdv
Copy link
Member

@bryevdv bryevdv commented Aug 16, 2016

it actually looks like there are binaries available... wouldn't it be easier to just package those, than build them?

@msarahan
Copy link
Contributor

@msarahan msarahan commented Aug 16, 2016

Sorry, it looks like your nodejs package built OK. I don't know what that "Cannot find module" message might come from. I have never used nodejs - only compiled it.

@bryevdv
Copy link
Member

@bryevdv bryevdv commented Aug 16, 2016

let me try 6.x, and then failing that, 4.4.7

@bryevdv
Copy link
Member

@bryevdv bryevdv commented Aug 16, 2016

OK I had some "globally" installed things from previously and I think this was the problem. I installed 6.3.1 and it seems to be working

@bryevdv
Copy link
Member

@bryevdv bryevdv commented Aug 16, 2016

@mattpap ou should be able to use the recipe above, otherwise I can build 6.3.1 on a linux vm tomorrow. I'm not sure what a windows pkg will entail

@mattpap mattpap added this to the 0.12.3 milestone Aug 29, 2016
@mattpap
Copy link
Contributor Author

@mattpap mattpap commented Aug 30, 2016

Sphinx error:
Unable to generate Bokeh plot at
/home/travis/build/bokeh/bokeh/sphinx/source/docs/user_guide/extensions.rst:194:<module 'builtin' > (built-in)> is a built-in class

Does anyone know what this means? This code works when run manually.

@bryevdv
Copy link
Member

@bryevdv bryevdv commented Aug 30, 2016

@mattpap I ran with SPHINXOPTS=-vv and got

Traceback (most recent call last):
  File "/Users/bryan/work/bokeh/bokeh/sphinxext/bokeh_plot.py", line 275, in html_visit_bokeh_plot
    js, script = autoload_static(plot, resources, filename)
  File "/Users/bryan/work/bokeh/bokeh/embed.py", line 335, in autoload_static
    js_raw = resources.js_raw + [script],
  File "/Users/bryan/work/bokeh/bokeh/resources.py", line 388, in js_raw
    custom_models = gen_custom_models_static()
  File "/Users/bryan/work/bokeh/bokeh/util/compiler.py", line 241, in gen_custom_models_static
    impl = model.implementation
  File "/Users/bryan/work/bokeh/bokeh/util/compiler.py", line 209, in implementation
    impl = impl.__class__(impl.code, self.file + ":" + self.name)
  File "/Users/bryan/work/bokeh/bokeh/util/compiler.py", line 192, in file
    return abspath(inspect.getfile(self.cls))
  File "/Users/bryan/anaconda/lib/python3.4/inspect.py", line 524, in getfile
    raise TypeError('{!r} is a built-in class'.format(object))
TypeError: <module 'builtins' (built-in)> is a built-in class
@mattpap
Copy link
Contributor Author

@mattpap mattpap commented Aug 30, 2016

@bryevdv, can we make showing tracebacks the default in travis ci?

@bryevdv
Copy link
Member

@bryevdv bryevdv commented Aug 30, 2016

@mattpap sure, we already catch and print exceptions:

https://github.com/bokeh/bokeh/blob/master/bokeh/sphinxext/bokeh_plot.py#L298-L299

I'm not sure why adding -vv makes spinx itself somehow output more verbose stack traces. But -vv is incredibly voluminous, I'd rather just make that except block spit out the stack trace itself, personally.

@bryevdv
Copy link
Member

@bryevdv bryevdv commented Aug 30, 2016

@mattpap actually SPHINXOPTS=-v also prints the stack trace, and is alot less verbose. If you just want to make that change that's fine with me.

@mattpap
Copy link
Contributor Author

@mattpap mattpap commented Aug 30, 2016

That works for me.

@mattpap
Copy link
Contributor Author

@mattpap mattpap commented Aug 31, 2016

TypeError: <module 'builtins' (built-in)> is a built-in class

The problem here is that sphinx extension uses compile() and eval() pair to execute Python code, but doesn't configure the environment sufficiently enough for bokeh.util.compiler to work properly. bokeh needs to know the path to a custom model to allow for JS module resolution, but that's not possible to obtain, because the default module is __builtin__ which doesn't configure __file__ (obviously), which is in turn needed by inspect.getfile().

If a path is known, then the problem can be fixed in the sphinx extensions by adding:

import sys, types
sys.modules["fake"] = types.ModuleType("fake")
sys.modules["fake"].__file__ = "path from bokeh-plot directive"
namespace = {"__name__": "fake"}

Otherwise we can configure __file__ to whatever current directory is. Alternatively, we may want to fix the problem in bokeh.util.compiler, either by not requiring known custom module location (needs major redesign of the code) or making it work harder to figure out location (use current directory, etc.).

@mattpap
Copy link
Contributor Author

@mattpap mattpap commented Aug 31, 2016

I wanted to test this but when locally running make html I'm getting:

Sphinx error:
Unable to generate Bokeh plot at bokeh/sphinx/source/docs/gallery/ggplot_density.rst:12:libgfortran.so.1: cannot open shared object file: No such file or directory
mattpap added 3 commits Aug 31, 2016
This happens in interactive sessions and compile().
It's useful to know __file__ in compile(), e.g. to be able to properly
resolve dependencies of custom models. This only works with bokeh-plot:
path-to-file.py.
@mattpap
Copy link
Contributor Author

@mattpap mattpap commented Aug 31, 2016

libgfortran.so.1: cannot open shared object file: No such file or directory

Apparently conda install version 3 and ggplot/seaborn require version 1.

if path is not None:
sys.modules["fake"] = types.ModuleType("fake")
sys.modules["fake"].__file__ = path
namespace["__name__"] = "fake"

This comment has been minimized.

@mattpap

mattpap Aug 31, 2016
Author Contributor

@bryevdv, please review this and the remainder of changes in this file.

This comment has been minimized.

@bryevdv

bryevdv Aug 31, 2016
Member

Roughly seems OK, I'd suggest building the docs and spot checking plots in various places (and all the extension ones).

As an aside I think almost all of this complicated code can be ripped out some day and replaced with a subprocess call to bokeh json and some trivial templating.

@mattpap mattpap merged commit 6aa55d9 into master Sep 7, 2016
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@mattpap mattpap deleted the mattpap/precompiler branch Sep 7, 2016
@bryevdv

This comment has been minimized.

Copy link
Member

@bryevdv bryevdv commented on efa5cf9 Mar 12, 2018

Hi, please use the mailing list (or SO) for support questions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

5 participants
You can’t perform that action at this time.