Hexbin recipe #2201
Hexbin recipe #2201
Conversation
|
As this is my first work in Makie, it would be great if someone experienced can have a look at this :) I am not quite sure, if I had done reasonable work with the axis :D |
|
Very cool, thank you, I'll try to take a look soon when I have some spare time. |
|
For some inputs I get an empty line (usually second to topmost it seems?) x = [-0.8229475043895498, -0.46621767925243984, -2.0720758196284805, 0.524687403763896, 0.9653685757558091, 1.7528735168451055, -0.33237984203429993, 0.3506865765367531, -0.8944929017899972, 0.2532684038441482]
y = [-1.7463655960215445, 1.8957111366590151, 0.5550468594781066, 1.2254393653690707, 1.371174685353981, -1.0206975175484125, 0.3360700428234833, 2.195537222554637, 0.9952661972027315, 0.28852172975641416]
hexbin(x, y)
|
| if (length(x)==0)|(length(y)==0) | ||
| return | ||
| end | ||
| axis = current_axis() |
There was a problem hiding this comment.
What exactly do you need the axis for? It looks like you're trying to access its limits? First of all, current_axis() isn't necessarily the axis you're currently plotting to, so you can't use that. Second, you're not supposed to access any axis stuff in a recipe, it lives a level below that in a simple Scene. (At least how recipes currently work). And third, you are only doing your calculation once, so if the limits changed the plot wouldn't update. You can access limits via the projection matrix of the Scene if you really need to (check out vlines etc for how it's done there) but it's kind of rare that we do this.
| function Makie.plot!(hb::Hexbin{<:Tuple{<:AbstractVector{<:Any}, <:AbstractVector{<:Any}}}) | ||
| x,y =hb[1:2] | ||
|
|
||
| polys = Observable(Polygon[]) |
There was a problem hiding this comment.
Polygon is an abstract type so this doesn't really help in terms of type stability.
There was a problem hiding this comment.
you could construct a fake poly and do typeof(fakepoly)[] if the type is hard to write manually (sometimes the case with GeometryBasics types I think)
| hex = [Polygon(hex_points.+p*Point2f(1/scaling_x,1/scaling_y)) for p in grid_points] | ||
|
|
||
| for b in hex | ||
| push!(polys[],b) |
There was a problem hiding this comment.
looks like you could also just append!(polys[], hex)? But anyway this would accumulate if you updated, so I'm not sure you want that
There was a problem hiding this comment.
ah I see now you empty the vector above
| calculate_grid(x[],y[],hb.gridsize[],hb.mincnt[],hb.scale[]) | ||
|
|
||
| replace_automatic!(hb,:colorrange)do | ||
| if isempty(count_hex[]) |
There was a problem hiding this comment.
not sure but this probably wouldn't update if you update the plot data
| (minimum(count_hex[]),maximum(count_hex[]) ) | ||
| end | ||
| end | ||
| if ! isempty(polys[]) |
There was a problem hiding this comment.
This is also written for a static plot. In Makie, we always try to make plots update-able if possible. So you would just plot polys one way or another, and if the data were updated such that there are now polygons, the plot would update as well.
| amount_hex=amount_hex[amount_hex.>=mincnt] | ||
| amount_hex = scale.(amount_hex) | ||
|
|
||
| xe = sin.(0:pi/3:5/3*pi) |
There was a problem hiding this comment.
you don't need to allocate the vectors here, you can loop over the range below and calculate x,y on the fly
|
|
||
| end | ||
| onany(calculate_grid,x,y,hb.gridsize, hb.mincnt,hb.scale) | ||
| calculate_grid(x[],y[],hb.gridsize[],hb.mincnt[],hb.scale[]) |
There was a problem hiding this comment.
this is done a little more easily by just saying notify(hb.gridsize) or whatever
| push!(polys[],b) | ||
| end | ||
| for a in amount_hex | ||
| push!(count_hex[],a) |
There was a problem hiding this comment.
so you're updating the vectors but never notify the observables again, so you wouldn't see an update in a live plot
|
Very good first pr, a lot of good things in there already! I made some initial comments, maybe you can clarify what the logic is supposed to be that you need limits for, I didn't quite understand from reading. |
|
Thanks a lot for the feedback! |
|
Yes I think it's better to decouple this from the axis limits, and have the user couple those if desired :) |
Description
Adds the recipe for hexbin plots (#368) with documentation.
Type of change
Checklist