add typed argument conversion (#3565)

* add typed argument conversion

* fix volume

* add function to get available conversions

* make conversion apply more narrowly

* more cleanly separate recursion in convert_arguments

* clean up

* allow to get axis before creating a plot

* clean up

GLMakie/src/screen.jl

@@ -497,7 +497,7 @@ function Base.delete!(screen::Screen, scene::Scene)
 
         # Remap scene IDs to a continuous range by replacing the largest ID
         # with the one that got removed
-        if deleted_id-1 != length(screen.screens)
+        if deleted_id - 1 != length(screen.screens)
             key, max_id = first(screen.screen2scene)
             for p in screen.screen2scene
                 if p[2] > max_id

MakieCore/src/conversion.jl

@@ -1,5 +1,4 @@
 
-function convert_arguments end
 
 """
     convert_attribute(value, attribute::Key[, plottype::Key])
@@ -31,8 +30,6 @@ struct NoConversion <: ConversionTrait end
 conversion_trait(::Type) = NoConversion()
 conversion_trait(T::Type, args...) = conversion_trait(T)
 
-convert_arguments(::NoConversion, args...) = args
-
 """
     PointBased() <: ConversionTrait
 
@@ -100,3 +97,116 @@ conversion_trait(::Type{<: Image}) = ImageLike()
 
 struct VolumeLike <: ConversionTrait end
 conversion_trait(::Type{<: Volume}) = VolumeLike()
+
+function convert_arguments end
+
+convert_arguments(::NoConversion, args...; kw...) = args
+
+"""
+    convert_arguments_typed(::Type{<: AbstractPlot}, args...)::NamedTuple
+
+Converts the arguments to their correct type for the Plot type.
+Throws appropriate errors if it can't convert.
+Returns:
+
+* `NoConversion()` if no conversion is defined for the plot type.
+* `args` untouched, if no conversion is requested via the `NoConversion` trait.
+* `NamedTuple` of the converted arguments, if a conversion is defined.
+"""
+function convert_arguments_typed end
+
+function convert_arguments_typed(P::Type{<:AbstractPlot}, @nospecialize(args...))
+    return convert_arguments_typed(typeof(conversion_trait(P, args...)), args...)
+end
+
+function convert_arguments_typed(::Type{<:ConversionTrait}, @nospecialize(args...))
+    # We currently just fall back to not doing anything if there isn't a convert_arguments_typed defined for certain plot types.
+    # This makes `@convert_target` an optional feature right now.
+    # It will require a bit more work to error here and request an overload for every plot type.
+    # We will have to think more about how to integrate it with user recipes, because otherwise all user defined recipes would error by default without a convert target.
+    # we return NoConversion to indicate, that no conversion is defined, while a conversion was requested, which is different from the below case where no conversion was requested.
+    return NoConversion()
+end
+
+# Explicitely requested no conversion (default for recipes)
+convert_arguments_typed(::NoConversion, args...) = args
+
+"""
+    @convert_target(expr)
+Allows to define a conversion target for a plot type, so that `convert_arguments` can be checked properly, if it converts to the correct types.
+Usage:
+```Julia
+@convert_target struct PointBased{N} # Can be the Plot type or a ConversionTrait
+    positions::AbstractVector{Point{N, Float32}}
+end
+```
+This defines an overload of `convert_arguments_typed` pretty much in this way (error handling etc omitted):
+```Julia
+function convert_arguments_typed(ct::Type{<: PointBased}, positions)
+    converted_positions = convert(AbstractVector{Point{N, Float32}} where N, positions)
+    return (positions = converted_positions,) # returns a NamedTuple corresponding to the layout of the struct
+end
+```
+This way, we can throw nice errors, if `convert_arguments` doesn't convert to `AbstractVector{Point{N, Float32}}`.
+Take a look at Makie/src/conversions.jl, to see a few of the core conversion targets.
+"""
+macro convert_target(struct_expr)
+    if !Meta.isexpr(struct_expr, :struct)
+        error("Expression must be `struct Target; fields...; end`")
+    else
+        target_name = struct_expr.args[2]
+
+        if Meta.isexpr(target_name, :curly)
+            target_name, targs... = target_name.args
+        else
+            targs = ()
+        end
+
+        body = struct_expr.args[3]
+        convert_expr = []
+        converted = Symbol[]
+        all_fields = filter(x -> !(x isa LineNumberNode), body.args)
+        if any(x -> !Meta.isexpr(x, :(::)), all_fields)
+            error("All fields need to be of type `name::Type`. Found: $(all_fields)")
+        end
+        names = map(x -> x.args[1], all_fields)
+        types = map(x -> :($(x.args[2]) where {$(targs...)}), all_fields)
+        for (name, TargetType) in zip(names, types)
+            conv_name = Symbol("converted_$name")
+            push!(converted, conv_name)
+            # We always add the where clause, since it's too complicated to match the static type parameters to the types that use them.
+            # This seems to work well, since Julia drops any unecessary where clause/ type parameter, while lowering.
+            # TODO figure out a way to drop the where close... Eval is the only thing I found, but shouldn't be used here.
+
+            expr = quote
+                # Unions etc don't work well with `convert(T, x)`, so we try not to convert if already the right type!
+                if $name isa $TargetType
+                    $conv_name = $name
+                else
+                    $conv_name = try
+                        convert($TargetType, $name)
+                    catch e
+                        arg_types = map(typeof, ($(names...),))
+                        err = """
+                        Can't convert argument $($(string(name)))::$(typeof($name)) to target type $($TargetType).
+                        Either define:
+                            convert_arguments(::Type{<: $($target_name)}, $(join(arg_types, ", "))) where {$($targs...)}) = ...
+                        Or use a type that can get converted to $($TargetType).
+                        """
+                        error(err)
+                    end
+                end
+            end
+            push!(convert_expr, expr)
+        end
+
+        expr = quote
+            function MakieCore.convert_arguments_typed(::Type{<:$(target_name)}, $(names...))
+                $(convert_expr...)
+                return NamedTuple{($(QuoteNode.(names)...),)}(($(converted...),))
+            end
+        end
+        return esc(expr)
+    end
+    return
+end

ReferenceTests/src/tests/figures_and_makielayout.jl

@@ -247,7 +247,7 @@ end
     values = [sin(x[i]) * cos(y[j]) * sin(z[k]) for i in 1:20, j in 1:20, k in 1:20]
 
     # TO not make this fail in CairoMakie, we dont actually plot the volume
-    _f, ax, cp = contour(x, y, z, values; levels=10, colormap=:viridis)
+    _f, ax, cp = contour(-1..1, -1..1, -1..1, values; levels=10, colormap=:viridis)
     Colorbar(fig[2, 1], cp; size=300)
 
     _f, ax, vs = volumeslices(x, y, z, values, colormap=:bluesreds)

src/Makie.jl

@@ -90,14 +90,16 @@ import MakieCore: plot, plot!, theme, plotfunc, plottype, merge_attributes!, cal
 import MakieCore: create_axis_like, create_axis_like!, figurelike_return, figurelike_return!
 import MakieCore: arrows, heatmap, image, lines, linesegments, mesh, meshscatter, poly, scatter, surface, text, volume
 import MakieCore: arrows!, heatmap!, image!, lines!, linesegments!, mesh!, meshscatter!, poly!, scatter!, surface!, text!, volume!
-import MakieCore: convert_arguments, convert_attribute, default_theme, conversion_trait
+import MakieCore: convert_arguments, convert_attribute, default_theme, conversion_trait, @convert_target,
+                  convert_arguments_typed
 
 export @L_str, @colorant_str
 export ConversionTrait, NoConversion, PointBased, GridBased, VertexGrid, CellGrid, ImageLike, VolumeLike
 export Pixel, px, Unit, plotkey, attributes, used_attributes
 export Linestyle
 
-const RealVector{T} = AbstractVector{T} where T <: Number
+const RealVector{T} = AbstractVector{T} where {T<:Real}
+const RealMatrix{T} = AbstractMatrix{T} where {T<:Real}
 const RGBAf = RGBA{Float32}
 const RGBf = RGB{Float32}
 const NativeFont = FreeTypeAbstraction.FTFont

src/basic_recipes/volumeslices.jl

@@ -17,7 +17,7 @@ $(ATTRIBUTES)
     )
 end
 
-function plot!(plot::VolumeSlices)
+function Makie.plot!(plot::VolumeSlices)
     @extract plot (x, y, z, volume)
     replace_automatic!(plot, :colorrange) do
         map(extrema, volume)