8000 Themed glyphs not applying · Issue #6821 · bokeh/bokeh · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
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.

Sign up for GitHub

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

Jump to bottom

Themed glyphs not applying #6821

Closed
bryevdv opened this issue Aug 27, 2017 · 13 comments
Closed

Themed glyphs not applying #6821

bryevdv opened this issue Aug 27, 2017 · 13 comments
Labels
reso: completed tag: component: bokehjs type: bug
Milestone
2.1

Comments

@bryevdv
Copy link
Member
bryevdv commented Aug 27, 2017

The text theme values in the code below are not applied.

from __future__ import print_function

import yaml

from bokeh.document import Document
from bokeh.models import (Circle, ColumnDataSource, DataRange1d, Grid, HoverTool, Label, LabelSet,
                          LinearAxis, Plot, Range1d, SingleIntervalTicker, CDSView, IndexFilter, Text)
from bokeh.resources import INLINE
from bokeh.sampledata.sprint import sprint
from bokeh.themes import Theme

# Based on http://www.nytimes.com/interactive/2012/08/05/sports/olympics/the-100-meter-dash-one-race-every-medalist-ever.html

abbrev_to_country = {
    "USA": "United States",
    "GBR": "Britain",
    "JAM": "Jamaica",
    "CAN": "Canada",
    "TRI": "Trinidad and Tobago",
    "AUS": "Australia",
    "GER": "Germany",
    "CUB": "Cuba",
    "NAM": "Namibia",
    "URS": "Soviet Union",
    "BAR": "Barbados",
    "BUL": "Bulgaria",
    "HUN": "Hungary",
    "NED": "Netherlands",
    "NZL": "New Zealand",
    "PAN": "Panama",
    "POR": "Portugal",
    "RSA": "South Africa",
    "EUA": "United Team of Germany",
}

fill_color = { "gold": "#efcf6d", "silver": "#cccccc", "bronze": "#c59e8a" }
line_color = { "gold": "#c8a850", "silver": "#b0b0b1", "bronze": "#98715d" }

def selected_name(name, medal, year):
    return name if medal == "gold" and year in [1988, 1968, 1936, 1896] else ""

t0 = sprint.Time[0]

sprint["Abbrev"]       = sprint.Country
sprint["Country"]      = sprint.Abbrev.map(lambda abbr: abbrev_to_country[abbr])
sprint["Medal"]        = sprint.Medal.map(lambda medal: medal.lower())
sprint["Speed"]        = 100.0/sprint.Time
sprint["MetersBack"]   = 100.0*(1.0 - t0/sprint.Time)
sprint["MedalFill"]    = sprint.Medal.map(lambda medal: fill_color[medal])
sprint["MedalLine"]    = sprint.Medal.map(lambda medal: line_color[medal])
sprint["SelectedName"] = sprint[["Name", "Medal", "Year"]].apply(tuple, axis=1).map(lambda args: selected_name(*args))

source = ColumnDataSource(sprint)

xdr =Range1d(start=25, end=-0.5)
ydr = DataRange1d(range_padding=2, range_padding_units="absolute")

plot = Plot(plot_width=1000, x_range=xdr, y_range=ydr, toolbar_location=None)
plot.title.text = "Usain Bolt vs. 116 years of Olympic sprinters"

xticker = SingleIntervalTicker(interval=5, num_minor_ticks=0)
xaxis = LinearAxis(ticker=xticker, axis_label="Meters behind 2012 Bolt")
plot.add_layout(xaxis, "below")

xgrid = Grid(dimension=0, ticker=xaxis.ticker)
plot.add_layout(xgrid)

yticker = SingleIntervalTicker(interval=12, num_minor_ticks=0)
yaxis = LinearAxis(ticker=yticker, major_tick_in=-5, major_tick_out=10)
plot.add_layout(yaxis, "right")

filters = [IndexFilter(list(sprint.query('Medal == "gold" and Year in [1988, 1968, 1936, 1896]').index))]

medal = Circle(x="MetersBack", y="Year", size=10, fill_color="MedalFill", line_color="MedalLine", fill_alpha=0.5)
medal_renderer = plot.add_glyph(source, medal)

#sprint[sprint.Medal=="gold" * sprint.Year in [1988, 1968, 1936, 1896]]
plot.add_glyph(source, Text(x="MetersBack", y="Year", x_offset=10, text="Name"), view=CDSView(source=source, filters=filters))

plot.add_glyph(source, Text(x=7.5, y=1942, text=["No Olympics in 1940 or 1944"],
                            text_font_style="italic", text_color="silver"))

tooltips = """
<div>
    <span style="font-size: 15px;">@Name</span>&nbsp;
    <span style="font-size: 10px; color: #666;">(@Abbrev)</span>
</div>
<div>
    <span style="font-size: 17px; font-weight: bold;">@Time{0.00}</span>&nbsp;
    <span style="font-size: 10px; color: #666;">@Year</span>
</div>
<div style="font-size: 11px; color: #666;">@{MetersBack}{0.00} meters behind</div>
"""

hover = HoverTool(tooltips=tooltips, renderers=[medal_renderer])
plot.add_tools(hover)

theme = Theme(json=yaml.load("""
attrs:
    Plot:
        outline_line_color: None
    Axis:
        axis_line_color: None
        major_tick_line_color: None
        axis_label_text_font_style: "bold"
        axis_label_text_font_size: "10pt"
    Grid:
        grid_line_dash: "dashed"
    Text:
        text_baseline: "middle"
        text_align: "center"
        text_font_size: "9pt"
"""))

doc = Document(theme=theme)
doc.add_root(plot)

if __name__ == "__main__":
    from bokeh.embed import file_html
    from bokeh.util.browser import view

    doc.validate()
    filename = "sprint.html"
    with open(filename, "w") as f:
        f.write(file_html(doc, INLINE, plot.title.text))
    print("Wrote %s" % filename)
    view(filename)
@bryevdv bryevdv added this to the 0.12.8 milestone Aug 27, 2017
@mattpap
Copy link
Contributor
mattpap commented Aug 27, 2017

btw. Updated version of this example is available in PR #6807. Perhaps it could be improved further based on code in this issue (e.g. usage of CDSView).

@bryevdv bryevdv modified the milestones: 0.13.x, short-term Sep 11, 2018
@bryevdv
Copy link
Member Author
bryevdv commented Mar 8, 2019

It would appear none of the theme is applied in 1.1dev

@bryevdv
Copy link
Member Author
bryevdv commented Nov 10, 2019

This keeps getting worse. Not only is none of the theme applied but the None color values trigger validation errors.

@bryevdv bryevdv modified the milestones: short-term, 2.0 Nov 10, 2019
@philippjfr philippjfr modified the milestones: 2.0, short-term Jan 15, 2020
@jeremy9959
Copy link
Contributor

I am interested in themes so I came across this issue. Was this tracked down to a problem on the javascript side?

@bryevdv
Copy link
Member Author
bryevdv commented Feb 19, 2020

I expect this is a problem on the Python side, but it will still require someone to investigate to uncover the root cause

@jeremy9959
Copy link
Contributor

I'll make a run at this.

@bryevdv
Copy link
Member Author
bryevdv commented Feb 19, 2020

👍 lmk if you need to have a call to do a deep dive on any of this. I'll probably have to remind myself of some things as I go through, for that matter. The theme implementation, is it relates to the larger property system and especially synchronization in the server case, is a bit involved. TLDR theme values need to be applied, but not in a way that triggers events and callbacks.

@jeremy9959
Copy link
Contributor
jeremy9959 commented Feb 19, 2020 •
edited
Loading

I can report some quick progress. There are two problems.

  • When you give "None" for a field, it gets encoded with value {'field':None}. If you specify the value you want explicitly, that fixes that problem.
  • You need to pass an explicit theme to file_html. Something is going wrong in the way themes are handled in that function.
    The following diff to the code example above makes it work:
diff fixed.py orig.py
101c101
<         outline_line_color: "#FFFFFF"
---
>         outline_line_color: None
103,104c103,104
<         axis_line_color: "#FFFFFF"
<         major_tick_line_color: "#FFFFFF"
---
>         axis_line_color: None
>         major_tick_line_color: None
125c125
<         f.write(file_html(doc, INLINE, plot.title.text, theme=doc.theme))
---
>         f.write(file_html(doc, INLINE, plot.title.text))

I'll look into file_html more closely.

@jeremy9959
Copy link
Contributor
jeremy9959 commented Feb 21, 2020 •
edited
Loading

I understand the problem now. When you call file_html(doc, INLINE, plot.title.text) without an explicit theme, the default behavior is to apply the theme from curdoc().
Here's what the documentation of theme argument to file_html says, emphasis mine.

theme (Theme, optional) :
            Defaults to the ``Theme`` instance in the **current document**.
            Setting this to ``None`` uses the default theme or the theme
            already specified in the document. Any other value must be an
            instance of the ``Theme`` class.

This is buried inside the context manager OutputDocumentFor in the method

def _set_temp_theme(doc, apply_theme):
    doc._old_theme = doc.theme
    if apply_theme is FromCurdoc:
        from ..io import curdoc; curdoc
        doc.theme = curdoc().theme
    elif apply_theme is not None:
        doc.theme = apply_theme

But in the situation we are looking at, curdoc() is not the same as doc and so the theme applied to doc is ignored -- it gets cached and then reapplied to doc in the ___exit___ of the context manager.

Weirdly, using the argument theme=None gives the correct result (as the documentation says).
So strictly speaking this code works as advertised, it's just somewhat obscure.

One fix is just to rename the theme argument to override_theme to better explain what's happening, and maybe change the default to None. 

override_theme (Theme, optional) :
            If set to a theme object, apply that  theme to the Document when creating html, 
            overriding the
            document theme attribute.  If set to ``None``,  use
            the default theme or the theme
            already specified in the document.  If set to ``FromCurdoc``
            apply the theme currently associated with the document returned by
            ``curdoc()``.

@jeremy9959
Copy link
Contributor

Last comment. You can't use

background_fill_color: None

in yaml if you want to set the value None. You need:

background_fill_color: !!null

Otherwise, None gets translated by the yaml parser as the string None, which the bokeh properties interpret as a field name. Again, not sure this is a bug, just a fact of life for yaml, but the documentation should make it clear.

@jeremy9959 jeremy9959 mentioned this issue Feb 22, 2020
Open
@bryevdv
Copy link
Member Author
bryevdv commented Feb 22, 2020

@jeremy9959 Thanks for looking in to this. I am not sure that curdoc() is really relevant outside the context of Bokeh application code, so perhaps that was an initial mistake in this codepath. Inside Bokeh apps, users need to be able to specify:

curdoc().theme = ....

and that's because inside the running app code, the curdoc() is exactly the Document that a session services. Outside a Bokeh app, Curdoc() is just (AFAIK) a random Document instance not attached to anything.

It think one approach, given the constraints, is to find a way for "standalone" functions like file_html to override or skip the curdoc check. Alternatively we could try to find a way to make curdoc() be None or otherwise markable as "not part of a real Bokeh app" so that the code you show above could ignore it. The latter is definitely a breaking change so we should probably look in to the former for now. Or if you have other thoughts/ideas please lmk.

This:

background_fill_color: !!null

is super unfortunate and ugly and I in fact did not know about it at all. Some googling seems to suggest just null would also work tho I did not test, and it's not much better. It would be good to consider adding documentation support around this specifically, and an example that demonstrates it.

@jeremy9959
Copy link
Contributor
jeremy9959 commented Feb 22, 2020 •
edited
Loading
8000

Changing the default value of theme in file_html to None from ```FromCurdoc`` results in the following behavior: Either you pass an explicit theme, or else:

  • If you call file_html with first argument either a document or the full set of models from a document, then the resulting output has the theme of that document applied.
  • If you call file_html with first argument a model, or a sequence of models that either come from different documents or are a proper subset of the models in a document, then the resulting output has the default theme applied.

So this change, plus a documentation update that explains this, seems like a minimal way to make this work. Before doing that I'd want to understand better how the other "standalone" functions work with themes and whether the same change should happen there.

My understanding is that the standalone functions embed stuff into pages that don't need a bokeh server to view. But curdoc comes from the server (is that right?) So when you say "this was an initial mistake in this codepath" do you mean that really the standalone functions shouldn't refer to curdoc at all?

This was referenced Feb 24, 2020
Merged
Merged
@bryevdv bryevdv modified the milestones: short-term, 2.1 May 25, 2020
@bryevdv bryevdv closed this as completed May 25, 2020
@github-actions GitHub Actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
reso: completed tag: component: bokehjs type: bug
Projects
None yet
Milestone
2.1
Development

N 30BA o branches or pull requests
4 participants
@mattpap @bryevdv @philippjfr @jeremy9959
0