xlabel! and ylabel! do not behave as expected

[czimm79]

When using xlabel! or ylabel! to set axis labels according to the docstring, Makie throws a method error.

using CairoMakie

lines(0:0.1:2π, sin)
xlabel!("My X Label")

MethodError: no method matching xlabel!(::String)

Closest candidates are:

xlabel!(::Any, !Matched::AbstractString)

In the source, xlabel! requires a scene positional argument. This conflicts with the docstring which specifies it as optional.

https://github.com/JuliaPlots/Makie.jl/blob/84a026f5a59ee71df50f2486be89da27a41af82d/src/shorthands.jl#L7-L18

From the documentation, it seems like scenes where phased out in favor of Figures. Also this discourse post further describes the same issue.

If this is a valid issue, I don't mind creating a draft pull request simply implementing a method for xlabel!(xlabel::AbstractString).

[yashi]

The commit 7882031 removed xlabel!(xlabel::AbstractString) and ylabel!(ylabel::AbstractString) but the docstrings haven't chnaged.

Unfortunately, even you call xlabel!(scene, xlabel::AbstractString), it doesn't seem to work, because scene[OldAxis] returns nothing.

Something like the following fixes but having this patch and realizing that the file name is src/shorthands.jl, it might be reasonable to think that xlabel!() is now deprecated and we should use axis.xlabel = "foo". What do you think?

diff --git a/src/shorthands.jl b/src/shorthands.jl
index 31fded1df..7a6546b2a 100644
--- a/src/shorthands.jl
+++ b/src/shorthands.jl
@@ -10,12 +10,12 @@ function zlims! end
 Set the x-axis label for the given Scene.
 Defaults to using the current Scene.
 """
-function xlabel!(scene, xlabel::AbstractString)
-    axis = scene[OldAxis]
+function xlabel!(axis::Axis, xlabel::AbstractString)
     @assert !isnothing(axis) "The Scene does not have an axis!"
-    scene[OldAxis][:names][:axisnames][] = (xlabel, scene[OldAxis][:names][:axisnames][][2:end]...)
-    nothing
+    axis.xlabel = xlabel
 end
+xlabel!(figure::Figure, xlabel::AbstractString) = xlabel!(current_axis(figure), xlabel)
+xlabel!(xlabel::AbstractString) = xlabel!(current_axis(), xlabel)
 
 """
     ylabel!([scene,] ylabel)

[czimm79]

I think that the fix you provide is appropriate, provided the docstring is also updated. Is there a way to hunt down the specific issue on why xlabel!(xlabel::AbstractString) was originally removed?

I don't think xlabel!() should be depreciated, as it is standard practice to use these shorthands for other things like xlims! and axislegend().

[yashi]

Is there a way to hunt down the specific issue on why xlabel!(xlabel::AbstractString) was originally removed?

I don't know. All I know is the commit, which is rather large and doesn't seem to explain why it did.
Does any of authors, @SimonDanisch, @ffreyer, @piever, @Datseris remember?

[jkrumbiegel]

The reason is that xlabel! was written for the old 2D axis that was only a plot object inside a scene, hence the scene argument. I wrote the new, much more complex Axis, but I didn't implement xlabel! for it, you just use ax.xlabel = .... Maybe we can just implement it or remove it entirely. I personally don't like duplication so much if it's not necessary.

[czimm79]

... but I didn't implement xlabel! for it, you just use ax.xlabel = .... Maybe we can just implement it or remove it entirely. I personally don't like duplication so much if it's not necessary.

I think no matter the fix, the interface should be consistent. If axis labels should be changed through ax.xlabel = ... syntax, xlims should be changed through ax.xlims = ... and xlims!() should be removed.

However, I think in a plotting library, duplication is necessary. The user doesn't really care how the xlabel is set, just that it works, and is simple to change. Multiple ways to do the same thing is fine and is common even in Makie.

[jkrumbiegel]

The comparison doesn't quite work because there is no xlims attribute, only limits, and xlims! sets half of it. I'm not sure what's the best way forward, in principle I think that it makes sense to use functions where attributes don't work and not duplicate the attributes. xlabel! would be a duplication of the xlabel attribute.

[czimm79]

I see what you mean.

I'm looking at this from the perspective of a naïve user. To explain why xlims! exists while xlabel! does not, I would need to know some deeper information on how Makie stores information. For example what you said - limits of the axis are stored in the limits attribute, which stores both xlims and ylims tuples, and that's why I need to use a separate function to modify one of the tuples.

[adamslc]

I got bitten by this today. What made it even more confusing is that there is an xlabel! function, so I assumed that it did what I wanted. I would humbly suggest either removing xlabel! or implementing it for Axis.

[jkrumbiegel]

@SimonDanisch maybe we should deprecate that function. We might want to replace the main LScene at some point with a FreeAxis3 or so, which just has normal top-level attributes like all the other blocks, so that you don't need xlabel! etc. like you did in the past. Currently that inner scene plus axis element logic is hard to understand