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

[FEATURE] Fail on bokeh.embed.file_html invalid template #9195

Open
unc0mm0n opened this issue Aug 28, 2019 · 3 comments
Open

[FEATURE] Fail on bokeh.embed.file_html invalid template #9195

unc0mm0n opened this issue Aug 28, 2019 · 3 comments

Comments

@unc0mm0n
Copy link

Is your feature request related to a problem? Please describe.
Right now bokeh.embed.file_html fails silently if given invalid input (e.g. file string "./template.html") as input parameter and generates with the default bokeh.core.template.FILE template. This leads mostly to confusion.

Describe the solution you'd like
Throw an exception when the parameter for template cannot be used, instead of silently deleting it.

Describe alternatives you've considered
Possibly print a warning message stating that template was swapped to default, or at least support creation of Jinja2 Templates from file name strings

@bryevdv
Copy link
Member

bryevdv commented Aug 28, 2019

@unc0mm0n the code actually ends up here:

    elif isinstance(template, string_types):
        template = _env.from_string("{% extends base %}\n" + template)

and here base=FILE. The intended capability here is to allow users to extend the FILE template, not replace it. The FILE template looks like this:

{% from macros import embed %}

<!DOCTYPE html>
<html lang="en">
  {% block head %}
  <head>
    {% block inner_head %}
      <meta charset="utf-8">
      <title>{% block title %}{{ title | e if title else "Bokeh Plot" }}{% endblock %}</title>
      {% block preamble %}{% endblock %}
      {% block resources %}
        {% block css_resources %}
          {{ bokeh_css | indent(8) if bokeh_css }}
        {% endblock %}
        {% block js_resources %}
          {{ bokeh_js | indent(8) if bokeh_js }}
        {% endblock %}
      {% endblock %}
      {% block postamble %}{% endblock %}
    {% endblock %}
  </head>
  {% endblock %}
  {% block body %}
  <body>
    {% block inner_body %}
      {% block contents %}
        {% for doc in docs %}
          {{ embed(doc) if doc.elementid }}
          {% for root in doc.roots %}
            {% block root scoped %}
              {{ embed(root) | indent(10) }}
            {% endblock %}
          {% endfor %}
        {% endfor %}
      {% endblock %}
      {{ plot_script | indent(8) }}
    {% endblock %}
  </body>
  {% endblock %}
</html>

So the way to use this is to supply template text that override, e.g. the "contents" block. I agree this is not well documented, so I will leave this issue open for that task.

I am not sure what else can be done, though. There's not 100% reliable way to judge if the string that is passed is a filename or not, which risks raising errors or warnings in cases where there should not be one.

@mattpap
Copy link
Contributor

mattpap commented Aug 28, 2019

I am not sure what else can be done, though. There's not 100% reliable way to judge if the string that is passed is a filename or not (...)

Starting with 2.0, we could use pathlib.Path to disambiguate strings and paths. This already proved very useful in e.g. django work.

@bryevdv
Copy link
Member

bryevdv commented Aug 28, 2019

That sounds good

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

No branches or pull requests

3 participants