add and export ReversibleScale type - simplify LogFunctions

[t-bltg]

Description

Fixes #3088, continuation of #2493 / #2900.

Add a new ReversibleScale type for custom scale functions (adds genericity in damping functions):

old examples

Example:

using CairoMakie

x = 10.0.^(1:0.1:4);
y = 1.0:0.1:5.0;
z = broadcast((x, y) -> x, x, y');

# forward and reverse scale function, with optional pseudo-log ticks
scale = ReversibleScale(x -> log10(x + 2), x -> exp10(x) - 2; logbase = "10");
axis = (; xscale = scale, xminorticksvisible = true, xminorticks = IntervalsBetween(9));

fig, ax, hm = heatmap(x, y, z; colorscale = scale, axis);
Colorbar(fig[1, 2], hm; minorticksvisible = true, minorticks = IntervalsBetween(9));

display(fig)
save("colorscale.png", fig)

Fix wrong algorithm in scaled_steps, which lead to log DomainErrors on the following example (on 0.19.7), added a new reference image:

using CairoMakie

x = 10.0.^(1:0.1:4);
y = 1.0:0.1:5.0;
z = broadcast((x, y) -> x, x, y');

axis = (; xscale = log10, xminorticksvisible = true, xminorticks = IntervalsBetween(9));
colorscale = ReversibleScale(x -> log10(x + 2), x -> exp10(x) - 2)

fig, ax, hm = heatmap(x, y, z; colorscale, axis);
Colorbar(fig[1, 2], hm; minorticksvisible = true, minorticks = IntervalsBetween(9));

display(fig)
save("colorscale-wrong.png", fig)

Without the ReversibleScale type, one would fail to determine the inverse transform function, hence leading to unoptimal colorbar ticks (root cause of the ticks overlap observed in #3088):

Simplify log functions declaration.

Fix #3089:

using CairoMakie

x = collect(1.0:10.0);
y = rand(10);

scale = ReversibleScale(log10; limits = (1, 10), interval = (0, Inf));
fig, ax, p = lines(x, y; axis = (; yscale = scale));

display(fig)
save("foo.png", fig)

Type of change

Delete options that do not apply:

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

• Added an entry in NEWS.md (for new features and breaking changes)
• Added or changed relevant sections in the documentation
• Added unit tests for new algorithms, conversion methods, etc.
• Added reference image tests for new plotting functions, recipes, visual options, etc.

[SimonDanisch]

Alright, CI is green! @jkrumbiegel do we want to merge this now? :)

[jkrumbiegel]

I have to take another look tomorrow, I just saw that the warning is still in there and I have to double check the logbase thing

[jkrumbiegel]

With this I get an empty colorbar (plus the warning):

f = Figure()
Colorbar(f[1, 1], limits = (1, 100), scale = (x -> log10(x)))
Colorbar(f[1, 2], limits = (1, 100), scale = log10)
f

I think it should probably not be possible to use a scale function without the correct inverse defined. I don't like this warning and I'm not sure the current design is what we really want.

Let's take a step back maybe and try to motivate again what we want, and why the current state of things is not enough. Is it the added convenience of the ReversibleScale struct over making a function plus the correct overloads? I've only seen the example with a log + offset function, are there other typical ones we don't cover?
If we want generality, why do we need the logbase field? This specializes the implementation on log-like scales, while we might rather want to have the ability to pass a tick formatter hint object, which might be used for other cases like sqrt or whatever there is out there.

I don't really like that it's so easy to pass a wrong log base to the scale, I'd rather want to have that determined either automatically (log2, log, log10) or done with a struct like LogTransform(some_value) where we automatically use some value, and the backwards transform is also automatic. We could even give LogTransform an offset value if that's commonly required.

You can also easily pass a mismatched pair of transforms, while that's also a "user error" thing I'm not sure if it's good.

f = Figure()
Colorbar(f[1, 1], limits = (1, 100), scale = Makie.ReversibleScale(sqrt, x->x^2))
Colorbar(f[1, 2], limits = (1, 100), scale = Makie.ReversibleScale(sqrt, exp10))
f

These things are hard to see in a final figure, so we should be careful and take things slow to get the implementation right.

[t-bltg]

Thanks for the comments.

I think it should probably not be possible to use a scale function without the correct inverse defined.

I don't see neither in which case one would use this, so let's hard error instead.

You can also easily pass a mismatched pair of transforms, while that's also a "user error" thing I'm not sure if it's good.

Since the given limits define a valid range, It's fairly easy to add a check cover this invalid usage: done in the latest commits.

If we want generality, why do we need the logbase field?

I agree that the logbase thing is a bit hacky, but I don't find an easy way to reproduce the first figure in #3095 (comment) without it. I can live without it, but I thought it would be nice for the log(... + offset) cases, which are really handy when visualizing data using non-linear transform functions.

where we automatically use some value, and the backwards transform is also automatic

Auto-determining the reverse scale is now implemented and tested.

[t-bltg]

Also, I don't have a strong opinion on the ReversibleScale name, so maybe there are better naming alternatives ?

[t-bltg]

@jkrumbiegel , #3194 seems to fail CI ?

[jkrumbiegel]

Ah yeah similar to the previous method, this one also doesn't work for external PRs because those users are not allowed to access the token needed. I'm not sure what to do to be honest, we can't comment, we can't status change, but we need to know if refimages are missing

[t-bltg]

Just to be clear, this pr doesn't have an added reference image, so the check should pass as before, no ?

[jkrumbiegel]

The behavior changed, now it sets a "success" status if no missing reference images were found, and that doesn't work here. We could remove that but then it still wouldn't work in cases with refimages.

[t-bltg]

I like to see these green statuses in the morning, awesome @jkrumbiegel !

[jkrumbiegel]

This run hopefully gives the status message. The last one was green but was missing the status (because getting the right commit SHA is not trivial in github actions..), but now I made changes on master that should resolve it correctly. We should see a "No new reference images" status here once the GLMakie CI runs finish.

The way I solved it now was to use a separate workflow which is only ever triggered from its default branch state, so can't be messed with by PRs. Therefore it is allowed to run at higher privileges, with access to status messages. The GLMakie CI stores the number of missing images and the right SHA in a file, which is downloaded by the separate workflow when GLMakie CI finishes. This one then posts the status message. 😌

[jkrumbiegel]

It worked 🎉

[t-bltg]

@jkrumbiegel, can we go forward on this PR ?

It would also simplify the contour labels example to avoid using the unexported Makie.sampler.

[jkrumbiegel]

Ok let's get this one in

[SimonDanisch]

Thank you all :)