From 930e3d459667139d3b6af3436165edd4f9715b7a Mon Sep 17 00:00:00 2001
From: jkrumbiegel <22495855+jkrumbiegel@users.noreply.github.com>
Date: Sat, 17 Apr 2021 10:35:28 +0200
Subject: [PATCH] jk/tick fixes (#695)

---
 src/makielayout/MakieLayout.jl |   3 +-
 src/makielayout/lineaxis.jl    | 143 ++++++++++++++-------------------
 src/makielayout/types.jl       |  21 ++++-
 test/unit_tests/makielayout.jl |  24 ++++++
 4 files changed, 108 insertions(+), 83 deletions(-)

diff --git a/src/makielayout/MakieLayout.jl b/src/makielayout/MakieLayout.jl
index d6a2056a..1bf67e6a 100644
--- a/src/makielayout/MakieLayout.jl
+++ b/src/makielayout/MakieLayout.jl
@@ -7,6 +7,7 @@ using ..AbstractPlotting.Keyboard
 using ..AbstractPlotting.Mouse
 using ..AbstractPlotting: ispressed, is_mouseinside, get_scene, FigureLike
 using ..AbstractPlotting: OpenInterval, Interval
+using ..AbstractPlotting: Automatic, automatic
 using Observables: onany
 import Observables
 import Formatting
@@ -84,7 +85,7 @@ export Textbox
 export linkxaxes!, linkyaxes!, linkaxes!
 export AxisAspect, DataAspect
 export autolimits!, limits!, reset_limits!
-export LinearTicks, WilkinsonTicks, MultiplesTicks, IntervalsBetween
+export LinearTicks, WilkinsonTicks, MultiplesTicks, IntervalsBetween, LogTicks
 export hidexdecorations!, hideydecorations!, hidedecorations!, hidespines!
 export tight_xticklabel_spacing!, tight_yticklabel_spacing!, tight_ticklabel_spacing!, tightlimits!
 export layoutscene
diff --git a/src/makielayout/lineaxis.jl b/src/makielayout/lineaxis.jl
index f2cae031..59d8c340 100644
--- a/src/makielayout/lineaxis.jl
+++ b/src/makielayout/lineaxis.jl
@@ -44,7 +44,7 @@ function LineAxis(parent::Scene; kwargs...)
     decorations[:minorticklines] = minorticklines
 
     realticklabelalign = lift(ticklabelalign, pos_extents_horizontal, flipped, ticklabelrotation, typ = Any) do al, (pos, ex, hor), fl, rot
-        if al !== AbstractPlotting.automatic
+        if al !== automatic
             return al
         end
         if rot == 0 || !(rot isa Real)
@@ -112,7 +112,7 @@ function LineAxis(parent::Scene; kwargs...)
     actual_ticklabelspace = attrs[:actual_ticklabelspace]
 
     onany(ticklabel_ideal_space, ticklabelspace) do idealspace, space
-        s = if space == AbstractPlotting.automatic
+        s = if space == automatic
             idealspace
         else
             space
@@ -437,21 +437,17 @@ function get_ticks(ticks, scale, formatter, vmin, vmax)
 end
 
 # automatic with identity scaling uses WilkinsonTicks by default
-get_tickvalues(::AbstractPlotting.Automatic, ::typeof(identity), vmin, vmax) = get_tickvalues(WilkinsonTicks(5, k_min = 3), vmin, vmax)
+get_tickvalues(::Automatic, ::typeof(identity), vmin, vmax) = get_tickvalues(WilkinsonTicks(5, k_min = 3), vmin, vmax)
 
 # fall back to identity if not overloaded scale function is used with automatic
-get_tickvalues(::AbstractPlotting.Automatic, F, vmin, vmax) = get_tickvalues(AbstractPlotting.automatic, identity, vmin, vmax)
+get_tickvalues(::Automatic, F, vmin, vmax) = get_tickvalues(automatic, identity, vmin, vmax)
 
 # fall back to non-scale aware behavior if no special version is overloaded
 get_tickvalues(ticks, scale, vmin, vmax) = get_tickvalues(ticks, vmin, vmax)
 
-# get_tickvalues(::AbstractPlotting.Automatic, ::typeof(log10), vmin, vmax) = get_tickvalues(Log10Ticks(), vmin, vmax)
 
-# get_tickvalues(::AbstractPlotting.Automatic, ::typeof(log2), vmin, vmax) = get_tickvalues(Log2Ticks(), vmin, vmax)
 
-# get_tickvalues(::AbstractPlotting.Automatic, ::typeof(log), vmin, vmax) = get_tickvalues(LogTicks(), vmin, vmax)
-
-function get_ticks(ticks_and_labels::Tuple{Any, Any}, any_scale, ::AbstractPlotting.Automatic, vmin, vmax)
+function get_ticks(ticks_and_labels::Tuple{Any, Any}, any_scale, ::Automatic, vmin, vmax)
     n1 = length(ticks_and_labels[1])
     n2 = length(ticks_and_labels[2])
     if n1 != n2
@@ -475,55 +471,51 @@ _logbase(::typeof(log10)) = "10"
 _logbase(::typeof(log2)) = "2"
 _logbase(::typeof(log)) = "e"
 
+
+function get_ticks(::Automatic, scale::Union{typeof(log10), typeof(log2), typeof(log)},
+        any_formatter, vmin, vmax)
+    get_ticks(LogTicks(WilkinsonTicks(5, k_min = 3)), scale, any_formatter, vmin, vmax)
+end
+
 # log ticks just use the normal pipeline but with log'd limits, then transform the labels 
-function get_ticks(x, scale::Union{typeof(log10), typeof(log2), typeof(log)}, any_formatter, vmin, vmax)
-    ticks_scaled = get_tickvalues(x, identity, scale(vmin), scale(vmax))
+function get_ticks(l::LogTicks, scale::Union{typeof(log10), typeof(log2), typeof(log)}, ::Automatic, vmin, vmax)
+    ticks_scaled = get_tickvalues(l.linear_ticks, identity, scale(vmin), scale(vmax))
     
     ticks = AbstractPlotting.inverse_transform(scale).(ticks_scaled)
 
-    if any_formatter === AbstractPlotting.automatic
-        # here we assume that the labels are normal numbers, and we just superscript them
-        labels_scaled = get_ticklabels(AbstractPlotting.automatic, ticks_scaled)
-        labels = _logbase(scale) .* AbstractPlotting.UnicodeFun.to_superscript.(labels_scaled)
-    else
-        # otherwise the formatter has to handle the real tick numbers
-        labels = get_ticklabels(any_formatter, ticks)
-    end
+    labels_scaled = get_ticklabels(automatic, ticks_scaled)
+    labels = _logbase(scale) .* AbstractPlotting.UnicodeFun.to_superscript.(labels_scaled)
 
     (ticks, labels)
 end
 
-# logit ticks
-function get_ticks(x, scale::typeof(AbstractPlotting.logit), any_formatter, vmin, vmax)
+# function get_ticks(::Automatic, scale::typeof(AbstractPlotting.logit), any_formatter, vmin, vmax)
+#     get_ticks(LogitTicks(WilkinsonTicks(5, k_min = 3)), scale, any_formatter, vmin, vmax)
+# end
+
+logit_10(x) = log10(x / (1 - x))
+expit_10(x) = 1 / (1 + exp10(-x))
 
-    logit_10(x) = log10(x / (1 - x))
-    expit_10(x) = 1 / (1 + exp10(-x))
-    ticks_scaled = get_tickvalues(x, identity, logit_10(vmin), logit_10(vmax))
+# function get_ticks(l::LogitTicks, scale::typeof(AbstractPlotting.logit), ::Automatic, vmin, vmax)
+
+#     ticks_scaled = get_tickvalues(l.linear_ticks, identity, logit_10(vmin), logit_10(vmax))
     
-    ticks = expit_10.(ticks_scaled)
-
-    if any_formatter === AbstractPlotting.automatic
-        base_labels = get_ticklabels(AbstractPlotting.automatic, ticks_scaled)
-        
-        labels = map(ticks_scaled, base_labels) do t, bl
-            if t == 0
-                "¹/₂"
-            elseif t < 0
-                "10" * AbstractPlotting.UnicodeFun.to_superscript(bl)
-            else
-                "1-10" * AbstractPlotting.UnicodeFun.to_superscript("-" * bl)
-            end
-        end
-        # # here we assume that the labels are normal numbers, and we just superscript them
-        # labels_scaled = get_ticklabels(AbstractPlotting.automatic, ticks_scaled)
-        # labels = _logbase(scale) .* AbstractPlotting.UnicodeFun.to_superscript.(labels_scaled)
-    else
-        # otherwise the formatter has to handle the real tick numbers
-        labels = get_ticklabels(any_formatter, ticks)
-    end
+#     ticks = expit_10.(ticks_scaled)
 
-    (ticks, labels)
-end
+#     base_labels = get_ticklabels(automatic, ticks_scaled)
+    
+#     labels = map(ticks_scaled, base_labels) do t, bl
+#         if t == 0
+#             "¹/₂"
+#         elseif t < 0
+#             "10" * AbstractPlotting.UnicodeFun.to_superscript(bl)
+#         else
+#             "1-10" * AbstractPlotting.UnicodeFun.to_superscript("-" * bl)
+#         end
+#     end
+
+#     (ticks, labels)
+# end
 
 """
     get_tickvalues(lt::LinearTicks, vmin, vmax)
@@ -533,31 +525,31 @@ Runs a common tick finding algorithm to as many ticks as requested by the
 """
 get_tickvalues(lt::LinearTicks, vmin, vmax) = locateticks(vmin, vmax, lt.n_ideal)
 
-# function get_tickvalues(::Log10Ticks, vmin, vmax)
-#     exp10.(ceil(log10(vmin)):floor(log10(vmax)))
-# end
-
-# function get_tickvalues(::Log2Ticks, vmin, vmax)
-#     exp2.(ceil(log2(vmin)):floor(log2(vmax)))
-# end
-
-# function get_tickvalues(::LogTicks, vmin, vmax)
-#     exp.(ceil(log(vmin)):floor(log(vmax)))
-# end
 
 """
     get_tickvalues(tickvalues, vmin, vmax)
 
 Convert tickvalues to a float array by default.
 """
-get_tickvalues(tickvalues, vmin, vmax) = Float64.(tickvalues)
+get_tickvalues(tickvalues, vmin, vmax) = convert(Vector{Float64}, tickvalues)
+
+
+# function get_tickvalues(l::LogitTicks, vmin, vmax)
+#     ticks_scaled = get_tickvalues(l.linear_ticks, identity, logit_10(vmin), logit_10(vmax))
+#     expit_10.(ticks_scaled)
+# end
+
+function get_tickvalues(l::LogTicks, scale, vmin, vmax)
+    ticks_scaled = get_tickvalues(l.linear_ticks, scale(vmin), scale(vmax))
+    AbstractPlotting.inverse_transform(scale).(ticks_scaled)
+end
 
 """
-    get_ticklabels(::AbstractPlotting.Automatic, values)
+    get_ticklabels(::Automatic, values)
 
 Gets tick labels by applying `Showoff.showoff` to `values`.
 """
-get_ticklabels(::AbstractPlotting.Automatic, values) = Showoff.showoff(values)
+get_ticklabels(::Automatic, values) = Showoff.showoff(values)
 
 """
     get_ticklabels(formatfunction::Function, values)
@@ -574,7 +566,7 @@ Gets tick labels by formatting each value in `values` according to a `Formatting
 get_ticklabels(formatstring::AbstractString, values) = [Formatting.format(formatstring, v) for v in values]
 
 
-function get_ticks(m::MultiplesTicks, any_scale, ::AbstractPlotting.Automatic, vmin, vmax)
+function get_ticks(m::MultiplesTicks, any_scale, ::Automatic, vmin, vmax)
     dvmin = vmin / m.multiple
     dvmax = vmax / m.multiple
     multiples = MakieLayout.get_tickvalues(LinearTicks(m.n_ideal), dvmin, dvmax)
@@ -584,7 +576,7 @@ end
 
 
 function get_minor_tickvalues(i::IntervalsBetween, scale, tickvalues, vmin, vmax)
-    vals = Float32[]
+    vals = Float64[]
     length(tickvalues) < 2 && return vals
     n = i.n
 
@@ -592,10 +584,7 @@ function get_minor_tickvalues(i::IntervalsBetween, scale, tickvalues, vmin, vmax
         firstinterval = tickvalues[2] - tickvalues[1]
         stepsize = firstinterval / n
         v = tickvalues[1] - stepsize
-        while v >= vmin
-            pushfirst!(vals, v)
-            v -= stepsize
-        end
+        prepend!(vals, v:-stepsize:vmin)
     end
 
     for (lo, hi) in zip(@view(tickvalues[1:end-1]), @view(tickvalues[2:end]))
@@ -612,18 +601,16 @@ function get_minor_tickvalues(i::IntervalsBetween, scale, tickvalues, vmin, vmax
         lastinterval = tickvalues[end] - tickvalues[end-1]
         stepsize = lastinterval / n
         v = tickvalues[end] + stepsize
-        while v <= vmax
-            push!(vals, v)
-            v += stepsize
-        end
+        append!(vals, v:stepsize:vmax)
     end
 
     vals
 end
 
 # for log scales, we need to step in log steps at the edges
-function get_minor_tickvalues(i::IntervalsBetween, scale::Union{typeof(log), typeof(log2), typeof(log10), typeof(AbstractPlotting.logit)}, tickvalues, vmin, vmax)
-    vals = Float32[]
+function get_minor_tickvalues(i::IntervalsBetween, scale::Union{typeof(log), typeof(log2), typeof(log10)}, tickvalues, vmin, vmax)
+
+    vals = Float64[]
     length(tickvalues) < 2 && return vals
     n = i.n
 
@@ -635,10 +622,7 @@ function get_minor_tickvalues(i::IntervalsBetween, scale::Union{typeof(log), typ
         prevtick = invscale(scale(tickvalues[1]) - firstinterval_scaled)
         stepsize = (tickvalues[1] - prevtick) / n
         v = tickvalues[1] - stepsize
-        while v >= vmin
-            pushfirst!(vals, v)
-            v -= stepsize
-        end
+        prepend!(vals, v:-stepsize:vmin)
     end
 
     for (lo, hi) in zip(@view(tickvalues[1:end-1]), @view(tickvalues[2:end]))
@@ -656,10 +640,7 @@ function get_minor_tickvalues(i::IntervalsBetween, scale::Union{typeof(log), typ
         nexttick = invscale(scale(tickvalues[end]) + lastinterval_scaled)
         stepsize = (nexttick - tickvalues[end]) / n
         v = tickvalues[end] + stepsize
-        while v <= vmax
-            push!(vals, v)
-            v += stepsize
-        end
+        append!(vals, v:stepsize:vmax)
     end
 
     vals
diff --git a/src/makielayout/types.jl b/src/makielayout/types.jl
index a20fdfd9..da2acc5c 100644
--- a/src/makielayout/types.jl
+++ b/src/makielayout/types.jl
@@ -50,7 +50,26 @@ struct MultiplesTicks
     suffix::String
 end
 
-struct Log10Ticks end
+
+# """
+#     LogitTicks{T}(linear_ticks::T)
+
+# Wraps any other tick object.
+# Used to apply a linear tick searching algorithm on a logit-transformed interval.
+# """
+# struct LogitTicks{T}
+#     linear_ticks::T
+# end
+
+"""
+    LogTicks{T}(linear_ticks::T)
+
+Wraps any other tick object.
+Used to apply a linear tick searching algorithm on a log-transformed interval.
+"""
+struct LogTicks{T}
+    linear_ticks::T
+end
 
 """
     IntervalsBetween(n::Int, mirror::Bool = true)
diff --git a/test/unit_tests/makielayout.jl b/test/unit_tests/makielayout.jl
index 25ca405e..e2039d50 100644
--- a/test/unit_tests/makielayout.jl
+++ b/test/unit_tests/makielayout.jl
@@ -103,4 +103,28 @@ end
             @test_throws ErrorException Colorbar(f[1, 3], p; Dict(attr => nothing)...)
         end
     end
+end
+
+@testset "Tick functions" begin
+    automatic = AbstractPlotting.automatic
+    Automatic = AbstractPlotting.Automatic
+
+    get_ticks = MakieLayout.get_ticks
+    get_tickvalues = MakieLayout.get_tickvalues
+    get_ticklabels = MakieLayout.get_ticklabels
+
+    for func in [identity, log, log2, log10, AbstractPlotting.logit]
+        tup = ([1, 2, 3], ["a", "b", "c"])
+        @test get_ticks(tup, func, automatic, 0, 5) == tup
+
+        rng = 1:5
+        @test get_ticks(rng, func, automatic, 0, 5) == ([1, 2, 3, 4, 5], ["1", "2", "3", "4", "5"])
+
+        numbers = [1.0, 1.5, 2.0]
+        @test get_ticks(numbers, func, automatic, 0, 5) == (numbers, ["1.0", "1.5", "2.0"])
+    
+        @test get_ticks(numbers, func, xs -> string.(xs) .* "kg", 0, 5) == (numbers, ["1.0kg", "1.5kg", "2.0kg"])
+
+        @test get_ticks(WilkinsonTicks(5), identity, automatic, 1, 5) == ([1, 2, 3, 4, 5], ["1", "2", "3", "4", "5"])
+    end
 end
\ No newline at end of file