Incorrect linestyle representation in legend for custom linestyles

[Datseris]

MWE:

f = Figure()
ax = Axis(f[1, 1])

xs = 0:0.01:10
ys = 0.5 .* sin.(xs)

lw = 3
i = 1
lines!(xs, ys .- i/6, linestyle = nothing, linewidth = lw, label = "$(lw)")
lines!(xs, ys .- i/6 .- 1, linestyle = :dash, linewidth = lw, label = "$(lw)")
lines!(xs, ys .- i/6 .- 2, linestyle = :dot, linewidth = lw, label = "$(lw)")
lines!(xs, ys .- i/6 .- 3, linestyle = :dashdot, linewidth = lw, label = "$(lw)")
lines!(xs, ys .- i/6 .- 4, linestyle = [0.0, 4.0, 6.0, 9.5], linewidth = lw, label = "$(lw)")
axislegend(ax)
f

as you can see, the legend incorrectly displays the linestyle: it doesn't have any dashes, but it is a continuous line.

[Datseris]

Notice also that the values I give to this four-element vector do not affect the display of the legend in any way

[jkrumbiegel]

the dash is probably just as long as the marker, increase patchsize to make the pattern visible

[jkrumbiegel]

if not, it's a bug

[Datseris]

this can't be it, changing to lines!(xs, ys .- i/6 .- 4, linestyle = [3.0, 4.0, 6.0, 6.5], linewidth = lw, label = "$(lw)") gives

you see the dashes are even more frequent than :dash style, but legend remains the same

[ffreyer]

axislegend is converting [0.0, 4.0, 6.0, 9.5] to nothing for me.

...
al = axislegend(ax)
display(f)
al.blockscene.children[1].plots[end].linestyle[] # nothing

[Datseris]

great, thanks, so this is clearly a bug then.

[jkrumbiegel]

Ah I think I understand what's happening, it's because the legend code assumes [0.0, 4.0, 6.0, 9.5] is a vector attribute, and therefore it doesn't pick a value for this attribute. I have thought before that this should probably be fixed by removing this kind of attribute for linestyle and replacing it by something more explicit, for example by wrapping with a struct.

[Datseris]

+1 for the struct, and also documenting what these values mean (coz I really have no clue!)

[ffreyer]

They're positions where the line flips from being drawn to not or vice versa, in units of linewidth. So with [0.0, 4.0, 6.0, 9.5] you start drawing at 0, stop at 4 linewidths, start again at 6, stop at 9.5, then repeat with 0 and 9.5 being treated as the same position.
In this case you effectively get a line that starts with a 4 linewidth drawn segment, and then 2 linewidth gaps with 7.5 linewidth drawn segments. You may see some artifacting with this in the drawn segments though, since the start and end of a segment try to anti-alias.

[Datseris]

Hi there @jkrumbiegel ; I'm currently preparing some figures for a publication that I'm about to submit, and I would really like to solve this issue as I need custom linestyles in the legend. No worries, I'm not expecting you to do it :P I'd like to solve this PR, but I would like some guidance first...

My plan:

1. Make a type CustomLinestyle(input::Vector{<:Real}). Its input is the same as current specification of linestyle.
2. Document and export it.
3. Deprecate passing vector as line style. I don't know how to do this.
4. Write a custom "extraction" function for the legend that if the input is CustomLinestyle, it does what it should. I think I would know how to do this if you point me to where in the source code a line object is transformed into a legend entry.

[jkrumbiegel]

1. sounds good, maybe even just Linestyle because there will be no other
2. ok
3. in the attribute conversions I'd think, probably in conversions.jl
4. that should just work
5. the backends would all need to know about the new type if we don't use a simple vector anymore. that's probably the biggest part to do though

[Datseris]

Okay, I now really need to fix this because our paper is about to be published so I'll get to work here!!!

@jkrumbiegel could I please ge tsome support here? I do not know what to do about part 5 nor where in the source code to add this new type CustomLinestyle.

[Datseris]

"""
    Linestyle(spec::Vector{<:Real})

A type that can be used as value for the `linestyle` keyword argument
of plotting functions to arbitrarily customize the linestyle.

The `spec` is a vector of positions where the line flips from being drawn or not
and vice versa. The values of `spec` are in units of linewidth. 

For example, with `spec = [0.0, 4.0, 6.0, 9.5]`
you start drawing at 0, stop at 4 linewidths, start again at 6, stop at 9.5, 
then repeat with 0 and 9.5 being treated as the same position.
"""
struct Linestyle
    spec::Vector{Float32}
end

type is here for reference.

[Datseris]

It appears that I should add this type here: https://github.com/MakieOrg/Makie.jl/blob/master/MakieCore/src/types.jl

[jkrumbiegel]

Yeah that location could work, or somewhere in Makie where other attribute types are defined. Then you'd define a method here https://github.com/MakieOrg/Makie.jl/blob/master/src/conversions.jl#L870 for this new type, create a plot and see where in the backend it errors because GLMakie etc. don't know about it. Alternatively, this type could get an attribute convert to the array form, then it might just work immediately. Although I was kind of hoping we could remove that one since we're not using arrays for scalar attributes anywhere else so I consider it to be a design wart. @SimonDanisch ?

[Datseris]

Are there any instructions anywhere on how to "develop" makie? Due to the mono repo structure, I do not know how I can do it, as I only know how to do dev PackageName and this will not work here as I need to simultaneously develop CairoMakie and Makie but they share the github repo.

[SimonDanisch]

]dev --local PackageName Checks out the Makie repository in the current directory (./dev/Makie).
Then you can do e.g.:
]dev ./dev/Makie/MakieCore ./dev/Makie/GLMakie
Or any other backend ;)

@jkrumbiegel I already replied to you, but the message is gone somehow -.- Ah well:

Although I was kind of hoping we could remove that one since we're not using arrays for scalar attributes anywhere else so

What do you want to remove? And I think linestyle is the only attribute left, where we use an array for scalar attributes, right?
@Datseris you should definitely define a convert_attribute, so that it works for all backends the same!
I don't think there is or should be another way ;)
Although we could probably improve the return type for linestyles, but that's another story!

[jkrumbiegel]

What do you want to remove?

@SimonDanisch the ability to give a linestyle as an array of numbers at all. If it's an array it should be an array of Linestyle objects or symbols for linesegments (it doesn't make sense for lines I think).

[Datseris]

OH MY DEAR LORD. GUYS. IT JUST WORKED. I cAN"T BELIEVE HOW EASY IT WAS:

# here CairoMakie, Makie, MakieCore are in development mode:

"""
    Linestyle(value::Vector{<:Real})

A type that can be used as value for the `linestyle` keyword argument
of plotting functions to arbitrarily customize the linestyle.

The `value` is a vector of positions where the line flips from being drawn or not
and vice versa. The values of `value` are in units of linewidth.

For example, with `value = [0.0, 4.0, 6.0, 9.5]`
you start drawing at 0, stop at 4 linewidths, start again at 6, stop at 9.5,
then repeat with 0 and 9.5 being treated as the same position.
"""
struct Linestyle
    value::Vector{Float32}
end

# Overload conversions
Makie.convert_attribute(A::Linestyle, ::key"linestyle") = [float(x - A.value[1]) for x in A.value]
# add deprecation for old conversion
function Makie.convert_attribute(A::AbstractVector, ::key"linestyle")
    @warn "Using a vector as a linestyle attribute is deprecated. Wrap it in a `Linestyle`."
    return [float(x - A[1]) for x in A]
end



# MWE:
f = Figure()
ax = Axis(f[1, 1])

xs = 0:0.01:10
ys = 0.5 .* sin.(xs)

lw = 3
i = 1
lines!(xs, ys .- i/6, linestyle = nothing, linewidth = lw, label = "$(lw)")
lines!(xs, ys .- i/6 .- 2, linestyle = :dot, linewidth = lw, label = "$(lw)")
lines!(xs, ys .- i/6 .- 4, linestyle = Linestyle([0.0, 4.0, 6.0, 9.5]), linewidth = lw, label = "$(lw)")
axislegend(ax)
display(f)

[Datseris]

[Datseris]

I am doing a PR right now!!!

[ffreyer]

axislegend is converting [0.0, 4.0, 6.0, 9.5] to nothing for me.

...
al = axislegend(ax)
display(f)
al.blockscene.children[1].plots[end].linestyle[] # nothing

Now it's :solid, so still buggy