fix error bar range in recipes

[palday]

Description

Fixes #3176

Moves to data space instead of pixel space.

Type of change

Delete options that do not apply:

• Bug fix (non-breaking change which fixes an issue)

Checklist

• Added an entry in NEWS.md (for new features and breaking changes)
• Added or changed relevant sections in the documentation
• Added unit tests for new algorithms, conversion methods, etc.
• Added reference image tests for new plotting functions, recipes, visual options, etc.

[ffreyer]

Rather than reverting this change we should probably have a custom data_limits method like bracket for now

Makie.jl/src/basic_recipes/bracket.jl

Lines 119 to 121 in a4af178

	data_limits(pl::Bracket) = mapreduce(union, pl[1][]) do points
	Rect3f([points...])
	end

and figure out a clean way to ignore non data space limits in Axis later, maybe as part of #3040.

[palday]

@ffreyer there's already a custom data_limits

Makie.jl/src/basic_recipes/error_and_rangebars.jl

Lines 282 to 285 in a4af178

	# ignore whiskers when determining data limits
	function data_limits(bars::Union{Errorbars, Rangebars})
	data_limits(bars.plots[1])
	end

but I don't know enough to know how to update it to fix this problem

[ffreyer]

Huh, why doesn't that work then? Is that not getting called when updating limits?

[ffreyer]

Oh ok, the problem comes from errorbars being wrapped in another plot recipe. In this case Axis calls

Makie.jl/src/layouting/data_limits.jl

Lines 170 to 172 in a4af178

	function data_limits(plot::AbstractPlot)
	limits_from_transformed_points(iterate_transformed(plot))
	end

which goes into

Makie.jl/src/layouting/data_limits.jl

Lines 136 to 144 in a4af178

	function iterate_transformed(plot)
	points = point_iterator(plot)
	t = transformation(plot)
	model = model_transform(t)
	# TODO: without this, axes with log scales error. Why?
	trans_func = identity # transform_func(t)
	# trans_func = identity
	iterate_transformed(points, model, to_value(get(plot, :space, :data)), trans_func)
	end

and further into

Makie.jl/src/layouting/data_limits.jl

Line 76 in a4af178

point_iterator(plot::Combined) = point_iterator(plot.plots)

skipping the custom data_limits.

So if we swap out the data_limits function for

function point_iterator(bars::Union{Errorbars, Rangebars})
    point_iterator(bars.plots[1])
end

we should be good. If that works could you skim through the other plot recipes in basic_plots and stats and replace the other overwrites as well?

[palday]

The only other use in basic_recipes was your example from bracket.jl and a keywarg to axis3d!:

Makie.jl/src/basic_recipes/axis.jl

Lines 346 to 348 in a4af178

	function axis3d!(scene::Scene, lims = data_limits(scene, p -> isaxis(p) || not_in_data_space(p)); kw...)
	axis3d!(scene, Attributes(), lims; ticks = (ranges = automatic, labels = automatic), kw...)
	end

In stats, we have:

Makie.jl/src/stats/hexbin.jl

Lines 65 to 75 in a4af178

	function data_limits(hb::Hexbin)
	bb = Rect3f(hb.plots[1][1][])
	fn(num::Real) = Float32(num)
	fn(tup::Union{Tuple,Vec2}) = Vec2f(tup...)
	ms = 2 .* fn(hb.plots[1].markersize[])
	nw = widths(bb) .+ (ms..., 0.0f0)
	no = bb.origin .- ((ms ./ 2.0f0)..., 0.0f0)
	return Rect3f(no, nw)
	end

Let me know if I should change either or both of those to be point_iterator instead of data_limits

[ffreyer]

axis3d! is fine, the other two would be good to change. For hexbin I think you can just call decompose(Point2f, Rect3f(no, nw)) to keep it simple

[jkrumbiegel]

I wonder if this makes sense

function iterate_transformed(plot) 
     points = point_iterator(plot) 
     t = transformation(plot) 
     model = model_transform(t) 
     # TODO: without this, axes with log scales error.  Why? 
     trans_func = identity # transform_func(t) 
     # trans_func = identity 
     iterate_transformed(points, model, to_value(get(plot, :space, :data)), trans_func) 
 end 

Because the transformation / model of a parent plot doesn't affect its children directly, does it? But we seem to want to transform by it (although not really as trans_func is set to identity for some reason)

Maybe this one

function data_limits(plot::AbstractPlot) 
     limits_from_transformed_points(iterate_transformed(plot)) 
 end 

needs to be rewritten so that it just combines the limits of its children, and doesn't iterate directly over them.

[ffreyer]

Yea, but I think it's better to have this be a quick fix and consider changes to data_limits more wholistically with #2881 / #3040.

[jkrumbiegel]

I'm ok with that

[SimonDanisch]

Thank you!