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

[BUG] grid_line_alpha value of '1.0' doesn't work but '0.99' does work #12658

Open
ribeirompl-soldersmith opened this issue Dec 5, 2022 · 8 comments
Labels
python Issues that should only require updating Python code type: bug
Milestone

Comments

@ribeirompl-soldersmith
Copy link

ribeirompl-soldersmith commented Dec 5, 2022

Software versions

Python version : 3.8.3 (default, Jul 2 2020, 17:30:36) [MSC v.1916 64 bit (AMD64)]
IPython version : 8.1.1
Tornado version : 6.1
Bokeh version : 3.0.2
BokehJS static path : .......\lib\site-packages\bokeh\server\static
node.js version : (not installed)
npm version : (not installed)
Operating system : Windows-10-10.0.19041-SP0

Expected behavior

When using the dark_minimal theme, and setting the grid_line_alpha value to 1 (or 1.0):
I would expect the grid line to become opaque.

Observed behavior

The grid line alpha does not change.

Example code

from bokeh.plotting import curdoc, figure, show

curdoc().theme = "dark_minimal"
fig = figure()
fig.line([1, 2, 3, 4], [1, 2, 3, 4])
fig.title = "Before (grid_line_alpha=0.25 from theme)"
show(fig)

fig.grid.grid_line_alpha = 1.0
fig.title = "After (grid_line_alpha=1.0)"
show(fig)

fig.grid.grid_line_alpha = 0.99
fig.title = "After (grid_line_alpha=0.99)"
show(fig)

I expect to see the "After" plot having a more opaque grid line, than the "Before" graph.
However, they look the same.

But, if I instead set the grid_line_alpha to 0.99, then it visibly changes.

Screenshots

image

@mattpap mattpap added python Issues that should only require updating Python code type: bug and removed TRIAGE labels Dec 5, 2022
@mattpap mattpap added this to the 3.1 milestone Dec 5, 2022
@mattpap
Copy link
Contributor

mattpap commented Dec 5, 2022

The default value is 1.0, so setting the property to the same value is a no-op. We need to mark such properties as "dirty", so that when applying a theme, the default-looking value is preferred over the themed default.

@bryevdv
Copy link
Member

bryevdv commented Dec 5, 2022

That was my first thought as well, but it's also a little weird. class_default and instance_default return the themed default, so this shouldn't be a no-op:

def class_default(self, cls: type[HasProps], *, no_eval: bool = False):
""" Get the default value for a specific subtype of ``HasProps``,
which may not be used for an individual instance.
Args:
cls (class) : The class to get the default value for.
no_eval (bool, optional) :
Whether to evaluate callables for defaults (default: False)
Returns:
object
"""
return self.property.themed_default(cls, self.name, None, no_eval=no_eval)
def instance_default(self, obj: HasProps) -> T:
""" Get the default value that will be used for a specific instance.
Args:
obj (HasProps) : The instance to get the default value for.
Returns:
object
"""
return self.property.themed_default(obj.__class__, self.name, obj.themed_values())

So maybe there a bug in that computation.

@bryevdv
Copy link
Member

bryevdv commented Dec 5, 2022

We need to mark such properties as "dirty"

@mattpap FYI there has also been some recent talk around supporting light/dark modes via themes, so we will want to make sure all requirements are considered and make sure everyone is on the same page. I'd advise making a discussion first before embarking on any wider or structural changes to properties (making an issue for light/dark is on my short-term to-do).

@mattpap
Copy link
Contributor

mattpap commented Dec 7, 2022

I don't see this bug as a theming issue, but as a simple bokeh doesn't respect user inputs type of issue.

@mattpap mattpap modified the milestones: 3.1, 3.2 Feb 10, 2023
@gauravk97
Copy link

Hi @bryevdv @mattpap ,

I was just checking the repository and came across this issue, I feel like picking it up. Should I go ahead and take a look at it? Asking here in case this issue is being fixed by someone already. What would you say?

@bryevdv
Copy link
Member

bryevdv commented Feb 14, 2023

@gauravk97 If you want to take a look at things, that's great, and we would be happy to receive and review any proposed solution in a PR. I should be up front that this issue intersects the low-level foundational property system, so it will require some extra care.

@gauravk97
Copy link

Sure, I was just itching to code some python. If you have some other beginner issue (for this repo), I'd be happy to look into that as well. Otherwise, I will continue checking this one.

@bryevdv
Copy link
Member

bryevdv commented Feb 14, 2023

@gauravk97 this one might be good to look at https://github.com/bokeh/bokeh/issues/12345 (text literal to try to avoid linking unrelated issues)

@mattpap mattpap modified the milestones: 3.2, 3.3 Jun 7, 2023
@mattpap mattpap modified the milestones: 3.3, 3.x Oct 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
python Issues that should only require updating Python code type: bug
Projects
None yet
Development

No branches or pull requests

4 participants