Extending recipes with new types doesn't always work

[aplavin]

Trying to make a proper recipe for stuff like lines(function) that plot the function over the whole axis x-range. Current hacky implementation at https://github.com/JuliaAPlavin/MakieExtra.jl/blob/master/src/axisfunction.jl.

However, following the docs and defining required functions I still get a conversion error:

function Makie.plot!(p::Lines{<:Tuple{Function}})
    # TODO
end

Makie.convert_arguments(::Type{<:Lines}, f::Function) = (f,)

lines(sin)

# ArgumentError: Conversion failed for MakieCore.Lines (With conversion trait MakieCore.PointBased()) with args: Tuple{typeof(sin)} .
# MakieCore.Lines requires to convert to argument types Tuple{AbstractVector{<:Union{GeometryBasics.Point2, GeometryBasics.Point3}}}, which convert_arguments didn't succeed in.
# To fix this overload convert_arguments(P, args...) for MakieCore.Lines or MakieCore.PointBased() and return an object of type Tuple{AbstractVector{<:Union{GeometryBasics.Point2, GeometryBasics.Point3}}}.`

How to define it properly?

[SimonDanisch]

function Makie.convert_arguments(P::Makie.PointBased, f::Function)
     x = ...
     y = f.(x)
     return convert_arguments(P, x, y)
end

[aplavin]

How to make x depend on the axis limits here?
If that's possible, great! I just didn't find a way to do that.

[ffreyer]

It's not. The main point of convert_arguments is to convert user data to a single format for a given plot type. Things like limits aren't necessary for that.

You also need to be careful with cyclical updates when doing things based on limits, because limits are based on positional data. https://github.com/MakieOrg/Makie.jl/blob/master/src/basic_recipes/hvlines.jl for example discards the dependent dimension in data_limits by setting it to NaN.

[aplavin]

You also need to be careful with cyclical updates when doing things based on limits,
... for example discards the dependent dimension in data_limits by setting it to NaN.

Ok, and that's what I want, and that's what I implemented in a hacky way (by overriding lines(::Function) etc) in MakieExtra.jl. For limits specifically, I just pass xautolimits=false – but data_limits would also work.

The "proper" way to do such a recipe would be to define plot! like this:

function Makie.plot!(p::Lines{<:Function})
    func = p[1]
    limits = Makie.projview_to_2d_limits(p)

    points = lift(p, func, limits) do func, limits
        xs = range(first.(extrema(limits))..., length = 400)
        Makie.Point2d.(xs, func.(xs))
    end

    lines!(p, p.attributes, points)
    return p
end

That's what I'm trying to do, but it doesn't work.
Is the documentation wrong/outdated and this isn't the right way to define plot recipes?

[SimonDanisch]

You mean:

Conversion failed for Lines (With conversion trait PointBased()) with args: Tuple{typeof(sin)} .

Conversions for primitive plot types are checked now, so you can't do this anymore.
This helps with better error messages and well defined plot objects for the backend.

I think you'll need a new recipe for this:

@recipe(InfLines, f) do scene 
    return Makie.default_theme(Lines, scene)
end

function Makie.plot!(p::InfLines)
    func = p.f
    limits = Makie.projview_to_2d_limits(p)

    points = lift(p, func, limits) do func, limits
        xs = range(first.(extrema(limits))..., length=400)
        Makie.Point2d.(xs, func.(xs))
    end

    lines!(p, p.attributes, points)
    return p
end

inflines(sin, color = :red, linewidth = 2)

It feels better this way, since strictly speaking you're pirating lines, since you own neither types, but of course naming is harder...

You could also do:

function Makie.plot!(p::Plot{plot, Tuple{F}}) where F <: Function
    func = p[1]
    limits = Makie.projview_to_2d_limits(p)
    points = lift(p, func, limits) do func, limits
        xs = range(first.(extrema(limits))..., length=400)
        Makie.Point2d.(xs, func.(xs))
    end
    lines!(p, p.attributes, points)
    return p
end

plot(sin, color = :red, linewidth = 2)

Not sure if I like that though... Maybe plot(Lines, sin)?

[aplavin]

@recipe(InfLines, f)

Yeah I kinda hoped to avoid defining new plot types like this... Even for 1d, one reasonably needs at least lines and band, then for 2d stuff like image and contour. All of them would need new names, weird when lines(func) is perfectly clear.

It feels better this way, since strictly speaking you're pirating lines, since you own neither types

That's pretty benign piracy, adding new functionality and not changing existing one. Longer term I hoped that such a recipe is possible and can be upstreamed to Makie :) Unfortunately currently it doesn't seem possible in a clean way, so I'll just keep the hacky but working implementation in MakieExtra.

[aplavin]

Conversions for primitive plot types are checked now, so you can't do this anymore.

More generally, can this check be avoided/overridden?
It's unfortunate that one cannot define stuff like plot!(p::Lines{MyType}) now.

This helps with better error messages and well defined plot objects for the backend.

If a plot! method is defined for the requested argument, then it's clear that the type is intended to be supported (no "error message" needed) and results in well defined objects for the backend in the end. Right?

[SimonDanisch]

If a plot! method is defined for the requested argument, then it's clear that the type is intended to be supported

We've been fighting with ill defined plot objects for a long time now, and the fact, that you can't rely on lineplot[1] isa Vector{Point} has complicated our whole pipeline by a lot and makes maintenance harder.

Sadly, lines(...) does create a Line(...) object, which we're trying now to better specify and check for correctness.

Maybe we can find a way to treat Line{<: Function} differently - although we also tried to remove more code relying on the actual argument parameters in the plot type for compile time (the plot object goes through many functions, that need unnecessary specialization on the argument parameter).

I guess we can find some better ways to make this possible, but it's certainly more complicated than it looks on first sight, when knowing all the constraints that go into the design decision.

[jkrumbiegel]

Couldn't this specific problem be solved with a recipe that internally uses whatever plot type you pass to it? Probably would be a little tricky to set it up performantly but then at least you wouldn't need one recipe per function. I like the separate-recipe route better than making lines(func) work because such a recipe would also need different attributes, like the sampling density or strategy, for example.

So something like @recipe(FuncPlot) and then

funcplot(sin, Lines) or funcplot(sin, cos, Heatmap)

[SimonDanisch]

Yeah, that's what I meant to propose with plot(Lines, sin)...

[aplavin]

So, this tutorial https://docs.makie.org/stable/tutorials/wrap-existing-recipe is outdated, right? (specifically the last section) That's where I got the idea that overloading plot!(::PlotType{<:Tuple{ArgType}}) should work.

[aplavin]

I like the separate-recipe route better than making lines(func) work because such a recipe would also need different attributes, like the sampling density or strategy, for example.

These attributes aren't special to lines(func) at all: they belong to lines(interval, func) which should just be called by lines(func) with all attributes propagates. That's what I do in https://github.com/JuliaAPlavin/MakieExtra.jl/blob/000000000e377e4e12a52f8c98f10cb17a4ebd63/src/axisfunction.jl#L8.

So something like @recipe(FuncPlot) and then funcplot(sin, Lines) or funcplot(sin, cos, Heatmap)

For this specific case, probably this could be fine... Although, a very unusual interface compared to every other plotting function in Makie.
But more generally, this approach doesn't play well with Makie recipe/conversion infrastructure: eg, if someone defines convert_arguments(::Lines, x::MyObject) = (x.function,) it won't propagate to this recipe. And that's the whole point of conversion recipes :)

we also tried to remove more code relying on the actual argument parameters in the plot type for compile time (the plot object goes through many functions, that need unnecessary specialization on the argument parameter).

Can't this be solved with nospecialize?