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

Sphinx bokeh_plot extension should work outside project docs #5955

Closed
nathanielatom opened this Issue Mar 6, 2017 · 17 comments

Comments

Projects
None yet
4 participants
@nathanielatom
Copy link
Contributor

nathanielatom commented Mar 6, 2017

When using the sphinxext.bokeh_plot directive with the example from the docs, the sphinx build executes successfully (after following #5951). The js script is generated and copied into SPHINX_DOCS_ROOT_PATH/build/html/scripts/bokeh-plot-index-inline-5.js.

However when the file SPHINX_DOCS_ROOT_PATH/build/html/index.html is opened locally in chrome, the plot doesn't render (although the code-block does), and the console gets the following error:
Failed to load resource: net::ERR_FILE_NOT_FOUND with a link to file:///scripts/bokeh-plot-index-inline-5.js.

When I edited line 122 of PYTHON_LIB_PATH/bokeh/sphinxext/bokeh_plot.py from script_path = join("/scripts", js_name) to script_path = join("scripts", js_name), the plot appeared. I don't know a lot about web development, so I don't know if removing the / would break documentation hosted on a server.

Version Info:

Python version : 3.6.0 | packaged by conda-forge | (default, Jan 14 2017, 03:13:00)
IPython version : 5.1.0
Bokeh version : 0.12.4
BokehJS static path : /Users/Atom/.anaconda/envs/py3/lib/python3.6/site-packages/bokeh/server/static
node.js version : v6.3.1
npm version : 3.10.3
jupyter --version : 4.2.1
jupyter notebook --version : 4.3.1
os version : macOS 10.12.3
browser verision: Google Chrome Version 56.0.2924.87 (64-bit)
sphinx version: 1.5.3

This is a really awesome sphinx extension!

@bryevdv

This comment has been minimized.

Copy link
Member

bryevdv commented Mar 6, 2017

AFAIK the docs will have to be served by a real HTTP server. That is why we have a simple Flask docserver.py to serve the _build/html directory. But python -m http.server in that directory works just as well.

@nathanielatom

This comment has been minimized.

Copy link
Contributor Author

nathanielatom commented Mar 6, 2017

Sorry, I didn't elaborate on my use case. I'm actually trying to build custom docs, not the bokeh ones. I realize this sphinx extension was designed primarily for building the bokeh docs, but with this small change it's also incredibly useful for building custom static docs.

From what I can tell from docserver.py, the / is required for served docs. Could we create a conf.py config option like bokeh_sphinxext_bokeh_plot_is_served which would default to True but allow the / to be omitted when False? Or maybe even a bokeh_sphinxext_bokeh_plot_script_path option which defaults to "/scripts".

I can submit a PR if this sounds good.

@bryevdv

This comment has been minimized.

Copy link
Member

bryevdv commented Mar 6, 2017

Sorry, I didn't elaborate on my use case. I'm actually trying to build custom docs, not the bokeh ones.

Right I understood that. I was suggesting that you do serve them, even locally, which is as simple as python -m http.server Documenting that usage seems much preferable, since it means there is nothing new to maintain and explain. Is there a reason why that would not work in your case?

@bryevdv

This comment has been minimized.

Copy link
Member

bryevdv commented Mar 6, 2017

Also are you testing the directive in a top level page? (I think you must be?) On a lark I tried building the docs with /"scripts" changed to "scripts" and it doesn't work for me either with the doc server or just opening pages directly in the browser because the location of the scripts is no longer relative to the location of the page that the directive is on, when you are in a deeper subdirectory in the docs tree.

To make "scripts" work in general, I think would mean making sure he .js files are output in different scripts subdirectories the build/html tree along side the pages that contain the directives. There would definitely be some advantages to that, but I don' t think it would not be a trivial to make that change. (The bookkeeping is already alot of work with just the one directory, and wed have to figure out how to "properly" generate output in the build dir, which probably means a new custom Builder) I would support a PR that did that, though.

@nathanielatom

This comment has been minimized.

Copy link
Contributor Author

nathanielatom commented Mar 6, 2017

I was suggesting that you do serve them, even locally, which is as simple as python -m http.server Documenting that usage seems much preferable, since it means there is nothing new to maintain and explain. Is there a reason why that would not work in your case?

I was originally thinking of sharing my custom documentation (with non-python users) by simply sharing the static html files, as that way I could avoid the need to maintain a server. However, after a bit of consideration and discussion with the users, we've decided that the cost of maintaining a documentation server is worth the benefits it provides. I had thought the original use case would be more common, but maybe not.

Also are you testing the directive in a top level page?

Yeah, good catch, it totally breaks subdirectories.

To make "scripts" work in general, I think would mean making sure he .js files are output in different scripts subdirectories the build/html tree along side the pages that contain the directives. There would definitely be some advantages to that

What would the other advantages be? It sounds like it would involve a lot of complexity.

@bryevdv

This comment has been minimized.

Copy link
Member

bryevdv commented Mar 6, 2017

What would the other advantages be?

Well being able to use relative links is in general a good thing. In addition to letting things work with static (non-served) resources, which I think would indeed be more user-friendly, it would let me simplify some currently gorpy things in our nginx config for the main docs site. I won't fault anyone else for not being motivated by that last, though. :)

It sounds like it would involve a lot of complexity.

It's mostly just bookkeeping, so that the directive can be a good citizen and cleanup properly after itself, or update properly when source files are out of date. I'd describe it as more tedious than complex, but still some work either way.

Or maybe I am overthinking, maybe there is some small set of changes here:

https://github.com/bokeh/bokeh/blob/master/bokeh/sphinxext/bokeh_plot.py#L268-L286

and wherever env.bokeh_plot_files is updated that would do. Anyway I wouldn't rate it a higher priority but if you are looking for a PR to work on, I'm certainly happy to help answer questions :)

@hallyourbase

This comment has been minimized.

Copy link
Contributor

hallyourbase commented Feb 7, 2018

I have a a bit of a hacky solution to make it work for my specific use case. Unfortunately I don't have a ton of time to find an elegant method worthy of a PR. What I ended up doing was:

  • Place a known string ($REL_PATH$) at the front of the path when the script was parsed in _process_script
  • finding the document depth in run by counting the '/' chars in env.docname
  • replace the tagged string with the appropriate number of '../' to find the script file relative to the current file containing the plot directive.

I am building my docs on Linux right now but it has been working with gh-pages with these changes. I can post more details on gist if it is any use to anyone.

@bryevdv

This comment has been minimized.

Copy link
Member

bryevdv commented Feb 7, 2018

@hallyourbase please do post, I definitely want to fix this up in the library itself, but it's one task of many

@hallyourbase

This comment has been minimized.

Copy link
Contributor

hallyourbase commented Feb 20, 2018

@bryevdv Here are the changes I had made. Like I said, not an elegant solution so I am hesitant to create a PR. Changes are in BokehPlotDirective.run() and _process_script(). I can edit it to highlight the changes more with comments. Forgive me if this is not the preferred method of submitting code, I'm new to contributing. If it was a more elegant and robust solution I would just do a PR so the diff is clear. This does still put all of the generated scripts into the script directory, it just sets a relative path in the generated html to that directory.

@hallyourbase

This comment has been minimized.

Copy link
Contributor

hallyourbase commented Apr 13, 2018

@bryevdv Any updates on this? Do you want me to make a PR as described above?

@bryevdv

This comment has been minimized.

Copy link
Member

bryevdv commented Apr 13, 2018

@hallyourbase If you submit a PR that will make it much easier to compare the changes, so I'd say yes, though it may be starting point for more discussion or changes

@hallyourbase

This comment has been minimized.

Copy link
Contributor

hallyourbase commented Apr 16, 2018

@bryevdv Roger that, thanks. Submitted a first cut at it. Not the most elegant solution but I wanted to get the conversation going.

@bryevdv

This comment has been minimized.

Copy link
Member

bryevdv commented Apr 18, 2018

OK the other PR has been merged and offers a possible immediate way forward, but leaving this issue open to try and tackle the broader task of making all file locations more flexible later (i.e. please be aware the method in the PR may change in the future)

@Benjamin-Lee

This comment has been minimized.

Copy link
Contributor

Benjamin-Lee commented Jul 8, 2018

This would be incredibly helpful since the extension doesn't seem to embed usable plots on ReadTheDocs and I hate the idea of using screenshots in my docs

@bryevdv

This comment has been minimized.

Copy link
Member

bryevdv commented Jul 8, 2018

@Benjamin-Lee are you interested in working on it? It's not likely that I will be able to prioritize this anytime soon over other tasks, but I suspect it would be approachable to any non-beginner python dev, and I'd be happy to help provide guidance.

@Benjamin-Lee

This comment has been minimized.

Copy link
Contributor

Benjamin-Lee commented Jul 8, 2018

Sure -- I've been able to get working plots in my Sphinx docs by embedding manually outputted HTML so I can take a stab at it when I get a chance. I'll need to into the details of how the extension works first.

@bryevdv

This comment has been minimized.

Copy link
Member

bryevdv commented Jul 12, 2018

@Benjamin-Lee Great, sure, most of the relevant code is in this file:

https://github.com/bokeh/bokeh/blob/master/bokeh/sphinxext/bokeh_plot.py

Things are a little complicated by the fact that there are three ways to include a plot:

  • inline code in .rst file
  • A py file in the docs source tree
  • A py file outside the docs source tree

You can see these branches commented in the run method of the Directive. However, all of these paths use _process_script to generate the JS files, and to compute a path to the JS files for the script tag. Everything goes in env.bokeh_plot_files[js_name] = (script, js, js_path, source) And at the end everything uses build_finished to copy the JS files to the specified paths in the output.

So what is the task? Mostly bookkeeping. Those two function _process_script and build_finished currently make a location for the scripts a a fixed absolute path location in /scripts, and then copy them to the output in that location at the end. We want to make it more sophisticated, and instead generate locations that are right next to the source file that has the plot directive in it, and then copy the js scripts there, instead of the absolute path.

I think the first part of that is just calling autoload_static(d.roots[0], resources, js_name) instead of with script_path which is script_path = join("/scripts", js_name). That will generate a <script> tag with e.g. src="foo.js". And then for that to work, the builder_finished function at the end needs to be able to copy the js file into the proper location right next to the HTML output. I imagine, _process_script will need to compute the location, and then builder_fnished will look it up in env.bokeh_plot_files[js_name]

@bryevdv bryevdv referenced this issue Feb 18, 2019

Merged

Bryanv/sphinxext #8665

2 of 2 tasks complete

@bryevdv bryevdv modified the milestones: short-term, 1.1 Feb 18, 2019

@bryevdv bryevdv changed the title sphinxext.bokeh_plot generates script but doesn't render plot locally bokeh_plot Sphinx extension should work better outside project docs Mar 24, 2019

@bryevdv bryevdv changed the title bokeh_plot Sphinx extension should work better outside project docs Sphinx bokeh_plot extension should work better outside project docs Mar 24, 2019

@bryevdv bryevdv changed the title Sphinx bokeh_plot extension should work better outside project docs Sphinx bokeh_plot extension should work outside project docs Mar 24, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.