vlines doesn't work with units

[jariji]

I want the axis created with unitful values to keep those units

using Unitful, GLMakie
using Unitful: s
julia> fig, ax, plt = scatter(rand(5)s, rand(5))

julia> vlines!(ax, rand(10)s)
ERROR: DimensionError:  and s are not dimensionally compatible.

Stacktrace:
  [1] #s103#141
    @ ~/.julia/packages/Unitful/6r8Hq/src/conversion.jl:7 [inlined]
  [2] var"#s103#141"(::Any, s::Any, t::Any)
    @ Unitful ./none:0
  [3] (::Core.GeneratedFunctionStub)(::UInt64, ::LineNumberNode, ::Any, ::Vararg{Any})
    @ Core ./boot.jl:602
  [4] uconvert(a::Unitful.FreeUnits{(), NoDims, nothing}, x::Quantity{Float64, 𝐓, Unitful.FreeUnits{(s,), 𝐓, nothing}})
    @ Unitful ~/.julia/packages/Unitful/6r8Hq/src/conversion.jl:72
  [5] convert(::Type{Float64}, y::Quantity{Float64, 𝐓, Unitful.FreeUnits{(s,), 𝐓, nothing}})
    @ Unitful ~/.julia/packages/Unitful/6r8Hq/src/conversion.jl:139
  [6] macro expansion
    @ ~/.julia/packages/StaticArraysCore/7xxEJ/src/StaticArraysCore.jl:88 [inlined]
  [7] convert_ntuple
    @ ~/.julia/packages/StaticArraysCore/7xxEJ/src/StaticArraysCore.jl:84 [inlined]
  [8] Point{2, Float64}(x::Tuple{Quantity{Float64, 𝐓, Unitful.FreeUnits{(s,), 𝐓, nothing}}, Float64})
    @ GeometryBasics ~/.julia/packages/GeometryBasics/ebXl0/src/fixed_arrays.jl:21
  [9] StaticArray
    @ ~/.julia/packages/StaticArrays/85pEu/src/convert.jl:165 [inlined]
 [10] (::Makie.var"#721#724"{…})(val::Quantity{…}, mi::Int64, ma::Int64)
    @ Makie ~/.julia/packages/Makie/We6MY/src/basic_recipes/hvlines.jl:71
 [11] macro expansion
    @ ~/.julia/packages/Makie/We6MY/src/utilities/utilities.jl:213 [inlined]
 [12] broadcast_foreach(::Makie.var"#721#724"{…}, ::Vector{…}, ::Int64, ::Int64)
    @ Makie ~/.julia/packages/Makie/We6MY/src/utilities/utilities.jl:199
 [13] (::Makie.var"#720#723"{…})(lims::GeometryBasics.HyperRectangle{…}, vals::Vector{…}, mi::Int64, ma::Int64, transf::Tuple{…})
    @ Makie ~/.julia/packages/Makie/We6MY/src/basic_recipes/hvlines.jl:60

not revert to working with unitless values. Ideally it would error on

julia> vlines!(ax, rand(10))
Plot{Makie.vlines, Tuple{Vector{Float64}}}

because the axis is unitful.

[ffreyer]

There are two parts to this not working, which more or less also apply to vspan/hspan and probably a number of other recipe plots. (I.e. this also applies to #4412)

The first point of failure is here: https://github.com/MakieOrg/Makie.jl/blob/d7d407e59495eeced22aa71fe73377310e4a1d75/src/dim-converts/dim-converts.jl#L177-L181 Because vlines only has one argument no dim conversion is attempted and thus the units do not get stripped, causing an error when converting to Point types. If we remove this check we get further. On the vlines level units now get handled correctly, convert_arguments() passes and plot!(::VLines) passes.

But then we run into the second problem - dim_converts trigger again on the linesegments plot created by vlines. This plot has no units but is inserted into an Axis with units. We view this as incompatible, so it fails. Previously this would have skipped dim converts due to the check we removed.

If we did the same for hlines we'd run into another issue: try_dim_convert assumes that plot.args[idx] and converts[idx] act in the same dimension. That's true for vlines where plot.args[1] is an x value, but not hlines where it is a y value.

What I think we need is:

1. A way for plots to communicate how plot.args map to spatial dimensions/dim_converts
2. A way to skip dim_conversions for subplots

(1) seems failry straightforward to me. We could just add a function that given an args index returns a dim_converts index. Since some plots can change x/y direction via attributes, we'd need those there.

For (2) I'm not sure what to do. If we assume that only users add dim_converts (Units, Dates, Categorical values) we could just restrict dim converts to the top level plot. If not, we'd need to mark child plots as "probably already converted", where numeric values are seen as converted and units etc as needing conversions.

[jkrumbiegel]

Note that I also have to do 1 in AlgebraOfGraphics, would be cool if I could completely avoid that because Makie has the required facilities included.

[jariji]

@ffreyer

If not, we'd need to mark child plots as "probably already converted", where numeric values are seen as converted and units etc as needing conversions.

Does this mean

fig, ax, plt = scatter(rand(5)s, rand(5))
vlines!(ax, rand(10))

would not error? I much prefer an error there since seconds and unitless numbers are not dimensionally compatible --- I use units to check my work.

[ffreyer]

Well that one actually doesn't error right now, but it would with those changes. Currently that bypasses unit conversions because of branch I pointed out and just continues as if rand(10) was the correct unit. Without that branch and with (1) the args passed to vlines would get processed by the unit conversion pipeline and error there. (2) would come after that to ensure that it doesn't get processed again.