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

Fixed bokeh FuncTickFormater #1010

Merged
merged 3 commits into from Dec 11, 2016
Merged

Fixed bokeh FuncTickFormater #1010

merged 3 commits into from Dec 11, 2016

Conversation

@philippjfr
Copy link
Member

@philippjfr 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 changed the title Fixed FuncTickFormater with py2js_tickformatter util and tests Fixed bokeh FuncTickFormater Dec 10, 2016
@philippjfr philippjfr added this to the v1.7.0 milestone Dec 10, 2016
@jlstevens
Copy link
Contributor

@jlstevens 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
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
@philippjfr
s3-reference-data-cache Test data is cached.
Details
@philippjfr
Copy link
Member Author

@philippjfr 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
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants