-
Notifications
You must be signed in to change notification settings - Fork 251
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
hexbin misalignment #1497
Comments
Ah, I forgot to comment in this issue after making the upstream PR. The linked Hexagons.jl PR (still awaiting review) seems to fix this issue. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Spoilers: I'm opening this issue even though I think it's partly a bug upstream in https://github.com/GiovineItalia/Hexagons.jl
In fact, I'm hoping to open a PR there, though it may take me a while as I have no idea what I'm doing! If someone wants to beat me to it, please do. I'm also not sure about the etiquette of the upstream change, though luckily Gadfly is Hexagon's only direct dependent according to JuliaHub.
Apparent bug: rendered hexbins are misaligned with input data. Here's a small reproducing example:
plot(x = [-10,0,10], y = [-10,0,10], Geom.hexbin(xbincount=4, ybincount=4))
Notice how no hexagon covers
(-10,-10)
or(10,10)
, and the bottom left hexagon covers no data points at all! It appears the hexagons have been shifted down and to the left from their proper positions.I believe the issue begins here: https://github.com/GiovineItalia/Gadfly.jl/blob/master/src/statistics.jl#L1249-L1251
An input
(x,y)
is mapped to cubic hexagon coordinates (of the containing hexagon) usingcube_round
fromHexagons.jl
. Unfortunately,cube_round
returns the cubic coordinates of the hexagon that contains an input(x,y)
assuming the zero-cubic-coordinate hexagon has xy-center(1,1)
. This is nasty, and exactly what my future upstream PR will hopefully fix. See this old issue: GiovineItalia/Hexagons.jl#13But when the hexagon-space coordinates are translated back to
(x,y)
coordinates a few lines later:The
(x,y)
offset corresponding to the center of the coordinate system does not correct for the implicit(1,1)
xy-center of the hexagon coordinate system used incube_round
.So the upstream fix is to make
cube_round
(andcenter
) from Hexagons ditch the (1,1) alignment, i.e. the origin of the cubic hexagonal grid system should, by default, coincide with the origin of the xy Cartesian plane. That upstream fix will, I weakly believe, fix this misalignment.As a final note, I think the Gadfly
statistics.jl
code could be made a bit more readable as follows, though (if I'm understanding correctly) the final results won't change at all:cube_round
should have first argumentx - (xmin + xspin/2)
, and likewise fory
. Note the additional parentheses relative to the current code. I.e. a pointx
is mapped to its relative offset from the x-axis midpoint of the data.center
should have its last argumentsxmin + xspan/2
and likewise fory
; note the change in sign.This suggested code and the current code perform equivalent coordinate conversions xy -> hex -> xy; the current code just has an unintuitive-to-me translation by
xspan
andyspan
and then undoes that translation.The text was updated successfully, but these errors were encountered: