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

Fixed bokeh FuncTickFormater #1010

Merged
merged 3 commits into from Dec 11, 2016

Conversation

Projects
None yet
2 participants
@philippjfr
Member

philippjfr commented Dec 10, 2016

As described in #885 in bokeh 0.12.3 the signature of python FuncTickFormatters was changed in a way that is inconsistent with formatters in holoviews. This PR adds a utility that compiles the python formatter with pyscript directly and then patches the generated JS code so the arg are made available in the namespace rather than being passed to the function. It's not exactly pretty but I don't see a way around it. I've also added some unit tests.

@philippjfr philippjfr added the bokeh label Dec 10, 2016

@philippjfr philippjfr requested a review from jlstevens Dec 10, 2016

@philippjfr philippjfr changed the title from Fixed FuncTickFormater with py2js_tickformatter util and tests to Fixed bokeh FuncTickFormater Dec 10, 2016

@philippjfr philippjfr added this to the v1.7.0 milestone Dec 10, 2016

@jlstevens

This comment has been minimized.

Member

jlstevens commented Dec 11, 2016

I've never been entirely happy with the flexx dependency but that isn't something newly introduced in this PR. The mix of Python inspection, flexx, Javascript and regular expressions is quite unholy but it at least this scary code has now been confined to its own function.

Just as a shot in the dark, is there any chance AST tricks could be applied to the python function before passing it to py2js instead of manipulating the javascript output? Not that such an approach would be much better!

As I know you are aware, this isn't a nice approach and I'm afraid I don't have any better suggestions right now. Having a working tick formatter for bokeh is more important than avoiding one nasty function and on that basis, I'll now merge.

@jlstevens jlstevens merged commit cb23e47 into master Dec 11, 2016

4 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls First build on functick_fix at 75.999%
Details
s3-reference-data-cache Test data is cached.
Details
@philippjfr

This comment has been minimized.

Member

philippjfr commented Dec 11, 2016

The mix of Python inspection, flexx, Javascript and regular expressions is quite unholy but it at least this scary code has now been confined to its own function.

Agreed, definitely not pretty.

Just as a shot in the dark, is there any chance AST tricks could be applied to the python function before passing it to py2js instead of manipulating the javascript output? Not that such an approach would be much better!

Looked into this briefly but found no nice approach that actually worked, especially across python versions.

@philippjfr philippjfr deleted the functick_fix branch Dec 11, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment