-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Add size
to Scatter
#13958
Add size
to Scatter
#13958
Conversation
I think the best solution here is for the scatter sub-glyphs to inherit size by default if the user does not override it explicitly. I.e. it should be possible but not required to override the main glyph value, for sub-glyphs. That's how other properties work for sub-glyphs. |
c292ab4
to
63955e5
Compare
I tried to solve the problem and updated the javascript code to inherit from the base glyph. For the example the visual output meets my expectations. from bokeh.models import Scatter, Circle, ColumnDataSource
from bokeh.resources import Resources
from bokeh.plotting import figure, show, output_notebook
source = ColumnDataSource(dict(xx=[1, 2, 3, 4, 5],yy=[2, 5, 8, 2, 7]))
plot = figure(width=400, height=400, tools="tap", title="Select a circle")
renderer = plot.scatter('xx', 'yy', size=50, source=source)
selected_scatter = Circle(fill_alpha=1, fill_color="firebrick", radius=0.1)
nonselected_scatter = Scatter(fill_alpha=0.2, fill_color="blue", line_color="firebrick")
renderer.selection_glyph = selected_scatter
renderer.nonselection_glyph = nonselected_scatter
show(plot) The problem is, that the models on the Python side are not updated and I don't know how I can solve this. @bryevdv and @mattpap Help is needed. Any suggestions on the code and how to solve this for python are welcome. I will try finish this PR afterwards. |
@mosc9575 thanks for your patience, please take a look at #13957 (comment) for what I think is a better short-term "python only" solution that does not burden the user with manually keeping properties in sync, and does not burden us with technical lifts. To fully implement this, we'll want to search and update all examples that set sub-glyphs using the |
@bryevdv I will try to do this task in the next few days. |
from bokeh.models import (BasicTickFormatter, ColumnDataSource, CustomJSTickFormatter, | ||
Div, FixedTicker, LinearAxis, LinearColorMapper, Range1d) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While working on some examples I also fixed some PEP-Warnings and had to rearange some lines for the code style checks. This is why some files have "many" changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would nice if we could enable the same style of imports, that we use under src/
, for when we need to import a lot of models, etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I change the style of the imports the pre-push-hook test_isort
fails.
We could add -m=3
to the line below
bokeh/tests/codebase/test_isort.py
Line 58 in df57080
proc = run(["isort", "--gitignore", "--diff", "-c", dir], capture_output=True) |
but this woul mean, we have to change all import in all files. This would be worth a new issue. Is it okey for you if I open a seperate PR, to make this change to all import in this project?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The import style under examples
was made different from that under src
on purpose. We can discuss whether that should be the case or not any longer, but we should not take any action regarding that in this PR. In any was there should be an issue to discuss and decide the course of action first.
selection_glyph=quad.clone(fill_color='blue'), | ||
nonselection_glyph=quad.clone(fill_color='gray'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With this change and adding the "TapTool" to the tools, this example makes more sense to me. In the newer version a click on a renderer has a visual effect.
@@ -369,7 +369,7 @@ def harea_stack(self, stackers, **kw): | |||
Examples: | |||
|
|||
Assuming a ``ColumnDataSource`` named ``source`` with columns | |||
*2016* and *2017*, then the following call to ``harea_stack`` will |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The consecutive "will" is fixed, but this fix is unrelated to the original issue.
rearange imports
This PR fixes the explicit selection glyph example by adding size parameter to the Scatter model, which is by default 4.
Here the result with the fix using bokeh 3.4.2 in a jupyter notebook after a click.