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

circle not circular #7218

Closed
tvaught opened this issue Nov 15, 2017 · 13 comments · Fixed by #7342
Closed

circle not circular #7218

tvaught opened this issue Nov 15, 2017 · 13 comments · Fixed by #7342

Comments

@tvaught
Copy link

tvaught commented Nov 15, 2017

Version Information:

On Mac OS X 10.13.1 (High Sierra)
Python: 2.7.13 |Continuum Analytics, Inc.| (default, Dec 20 2016, 23:05:08)
[GCC 4.2.1 Compatible Apple LLVM 6.0 (clang-600.0.57)]
IPython: 5.3.0
Bokeh: 0.12.10

Problem:

When using p.circle(...), the rendered circle is not perfectly circular. The x dimension seems to render according to the supplied radius argument, but the y dimension is visibly short of the specified radius.

To reproduce:

The following code creates the screenshot below:

    from bokeh.plotting import figure, output_file, show

    output_file("circle_v1.html")

    p = figure(plot_width=400, plot_height=400, match_aspect=True, tools='save')
    p.circle(0, 0, radius=1.0, fill_alpha=0)
    p.line((0, 1), (0, 0))
    p.line((0, -1), (0, 0))
    p.line((0, 0), (0, 1))
    p.line((0, 0), (0, -1))
    #p.rect(0, 0, 2, 2, fill_alpha=0)

    show(p)

bokeh_plot 9

Note: When you uncomment the line beginning with #p.rect(... the dimensions of the rendered circle seem fine. e.g.:

bokeh_plot 10

Best,

Travis

@mattpap
Copy link
Contributor

mattpap commented Nov 15, 2017

When you uncomment the line beginning with #p.rect(... the dimensions of the rendered circle seem fine.

That's just a coincidence that comes from this particular setup of ranges.

@bryevdv
Copy link
Member

bryevdv commented Nov 15, 2017

That's just a coincidence that comes from this particular setup of ranges.

match_aspect with data ranges should work in both cases.

@mattpap
Copy link
Contributor

mattpap commented Nov 15, 2017

Yeah, I thought that match_aspect does something different (acts on frame dimensions).

@bryevdv
Copy link
Member

bryevdv commented Nov 15, 2017

I think perhaps some glyphs are not reporting their bounds very well, causing match_aspect to do the right thing with the wrong information

@bryevdv bryevdv added this to the 0.12.x milestone Nov 15, 2017
@tvaught
Copy link
Author

tvaught commented Nov 15, 2017 via email

@mansenfranzen
Copy link
Contributor

mansenfranzen commented Dec 3, 2017

Perhaps the following observation is also of some help: as soon as you manually adjust the x or y range either by p=figure(x_range=Range1D()) or p.x_range.start=-1 and p.x_range.end=1 it will overwrite what match_aspect does.

I'm still having trouble to understand why circle and line glyphs align differently on the same axis coordinate system. Even if the data aspect ratio and the screen aspect ratio vary independently, shouldn't it be the same for all glyphs?

@bryevdv
Copy link
Member

bryevdv commented Dec 3, 2017

@mansenfranzen that is entirely expected. Aspect matching only applies to auto ranging a with DataRange1d. If you manually and explicitly override things, Bokeh assumes you know what you are doing and honors the values you set.

As for the second, do you mean in general? It's because the low level canvas always draws actual circles regardless of the aspect ratio (i.e. nothing automatically squeezes it in to an ellipse,which is what a purely mathematical viewpoint would dictate). There is much discussion in old issues please refer to them.

@mansenfranzen
Copy link
Contributor

@bryevdv Thanks for the clarification. I finally got the point that the low level canvas draws circles irrespective of the aspect ratio. It explains the different behavior between rect and circle. I'm curious to know how other plotting projects employing the low level canvas deal with such challenges. Do you have any experiences on that?

@bryevdv bryevdv modified the milestones: 0.12.12, 0.12.13, 0.12.14 Dec 5, 2017
@bryevdv
Copy link
Member

bryevdv commented Dec 20, 2017

I'm curious to know how other plotting projects employing the low level canvas deal with such challenges. Do you have any experiences on that?

I was a bit cursory in my response. It is in fact possible to apply a scale transform to get the canvas to draw ellipses according to aspect at a low level. So the question becomes: is it appropriate to do so? The vast majority of uses of circle are for scatter plots, in which you always want small, actual circles, regardless of aspect, and thus the answer is "probably not really, definitely not in general" The issue really only affects "big" circles, in which case there is match_aspect (modulo bugs like this) or the ellipse glyph method should suffice. It's hard to justify adding more complexity to circle for configurable "auto-squashing", which is already the most complicated glyph by far (maybe a surprise but it has many applications and corner cases, this would complicate half a dozen hit testing code paths as an example).

@bryevdv
Copy link
Member

bryevdv commented Dec 20, 2017

OK I think I know why this is happening. First note that the following code does work:

from bokeh.plotting import figure, output_file, show

output_file("circle_v1.html")

p = figure(plot_width=400, plot_height=400, match_aspect=True, tools='save')
p.circle(0, 0, radius=1.0, fill_alpha=0)
p.line((-1, 1), (0, 0))
p.line((0, 0), (1, -1))

show(p) 

screen shot 2017-12-20 at 15 48 42

The reason for this (I believe) is that the match aspect adjustment is being made too early. The plot canvas collects all the glyph reported bounds, then applies match aspect to all of them separately. Then later the data range unions all the reported bounds, but only the ones that are in it's renderers lists. @tvaught It just happens that by choosing to split the two "diameters" into four "radiuses" exposes the issue, because the match aspect is not happening on the aggregated bounds for all the lines.

So the somewhat good news is that I think things will work in many common situations. The better news is that I think the solution is just to make DataRange1d responsible for applying aspect matching computations after union the culled bounds, which should not be too difficult (and better anyway)

@bryevdv
Copy link
Member

bryevdv commented Dec 21, 2017

OK I will push PR soon. The changes make the original code work as expected. I do think there are some caveats, in particular I think use of match_aspect and linked data ranges in multiple plots is probably not going to result in well-defined behavior. At this point I will just add a caution about that since I think trying to make that work would be quite alot of work.

@mansenfranzen
Copy link
Contributor

So the question becomes: is it appropriate to do so? The vast majority of uses of circle are for scatter plots, in which you always want small, actual circles, regardless of aspect, and thus the answer is "probably not really, definitely not in general"

@bryevdv I understand that in most cases circles are actually used as true circles irrespective of aspect ratio as in scatter plots. However, when composing glyphs to create new plots Iike a radial heatmap (like this PR in holoviews) , I expect glyphs to behave consistently on the same coordinate system. In the mentioned PR, it's not just circle but also annular_wedge glyphs which are are affected (probably because wedges use circles too).

I really love bokeh and the interactive/linked plots it allows to create but the inconsistent behavior of low level glyphs (in this case only circular glyphs) forced me thinking about using other plotting libraries (in the case of radial heatmaps). I guess new users who want to create new plots themselves would also expect circles and any other glyph to respect the aspect ratio.

I would love to see circular glyphs being able respect aspect ratios (if required to be set as a parameter/option). Yet, I have no clue where to start in order to get my head around how to accomplish this. Do you have some hints where I could start to look at? Perhaps I can be of a little help here.

@bryevdv
Copy link
Member

bryevdv commented Dec 26, 2017

@mansenfranzen that's what match_aspect is for (modulo actual bugs, like this one). Are you saying that it does not work with other glyphs? if so, a (new) GH issue would be appropriate (with complete reproducing code)

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

Successfully merging a pull request may close this issue.

4 participants