Skip to content

Fix automatic unit decision for Unitful axes #4940

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jusack
Copy link

@jusack jusack commented Apr 25, 2025

Description

Fixes the automatically determined units for Unitful axes that are symmetric.

Currently if an axis has the same absolute value for its positive and negative extrema, the determination of the best unit in Makie.best_unit gives the smallest possible prefix. E.g. Makie.best_unit(-1.0u"mm", 1.0u"mm") gives ym=yoctometer=10^-24 m because it only uses the value in the middle between min and max which is zero in this case.
This results in useless units for these types of axes:

lines(range(start=-1.0u"mm", stop=1.0u"mm", length=50), randn(50))

grafik
For a non-symmetric axis everything is fine

lines(range(start=0.0u"mm", stop=1.0u"mm", length=50), randn(50))

grafik

Proposed fix

In this PR I changed the decision of the unit to not use the middle value but instead the total length of the axis, which should be a characteristic value based on which the correct unit can be determined:
grafik
grafik

There are cases where the automatically decided axis unit is different than before, but I do not think this would count as a breaking change.
Before:

Makie.best_unit(0u"mm",10u"mm") # mm

This PR:

Makie.best_unit(0u"mm",10u"mm") # cm

Type of change

Delete options that do not apply:

  • Bug fix (non-breaking change which fixes an issue)

@github-project-automation github-project-automation bot moved this to Work in progress in PR review Apr 25, 2025
@jusack
Copy link
Author

jusack commented Apr 25, 2025

I see that the reference images actually hit one case where the automatic unit is different now. How should I proceed here?

@ffreyer
Copy link
Collaborator

ffreyer commented Apr 26, 2025

I'm not sure if the length would be a good metric either. Something like 1km .. 1km + 10cm would show up as 1000 00cm .. 1000 10cm then, right? Maybe 0.5 * (abs(low) + abs(high)) would be better?

@jusack
Copy link
Author

jusack commented Apr 28, 2025

Hm, that´s a good point I have not considered yet. Axes where the length greatly differs from the order of magnitude of the values are in general difficult to plot. One could argue for both the larger or the smaller unit, depending on which property is more important in that axis, the difference between two samples, or the overall size.

Using your example these are basically the possible options:

x = range(1.0u"km", 1.0u"km"+10u"cm", length=50); y=randn(50);
fig = Figure()
ax1 = Axis(fig[1,1], dim1_conversion=Makie.UnitfulConversion(u"km"))
ax2 = Axis(fig[1,2], dim1_conversion=Makie.UnitfulConversion(u"m"))
ax3 = Axis(fig[2,1], dim1_conversion=Makie.UnitfulConversion(u"cm"))
ax4 = Axis(fig[2,2], dim1_conversion=Makie.UnitfulConversion(u"mm"))
for ax in [ax1,ax2,ax3,ax4]
    lines!(ax,x,y)
end

grafik

In this case I would manually choose the m scale as its a tradeoff between both, however to automate that we would have to further extend the logic from using a single value.

For axes with even more orders of magnitude between offset and length it gets more difficult:

x = range(1.0u"km", 1.0u"km"+10u"µm", length=50); y=randn(50);
fig = Figure()
ax1 = Axis(fig[1,1], dim1_conversion=Makie.UnitfulConversion(u"km"))
ax2 = Axis(fig[1,2], dim1_conversion=Makie.UnitfulConversion(u"m"))
ax3 = Axis(fig[2,1], dim1_conversion=Makie.UnitfulConversion(u"mm"))
ax4 = Axis(fig[2,2], dim1_conversion=Makie.UnitfulConversion(u"µm"))
for ax in [ax1,ax2,ax3,ax4]
    lines!(ax,x,y)
end

grafik

I would say its good enough to have a default thats not completely off and for more complex axes the user has to manually change the units, depending on the use case.

My preference for a default would be a unit based on the length of the axis, as then you can see the scale of the steps at the first glance.

@icweaver
Copy link
Contributor

icweaver commented May 13, 2025

Yea, I've also had mixed feelings about best_unit and friends (SymbolicML/DynamicQuantities.jl#165 (comment)) lately. I was wondering how much the added complexity buys us over just keeping things explicit

For reference, here's how that might look with DynamicQuantities over in that PR:

import DynamicQuantities as DQ
using CairoMakie

lines(range(start=-1.0*DQ.us"mm", stop=1.0*DQ.us"mm", length=50), randn(50))

# Or with axis conversion API
#const DQConversion = Base.get_extension(DQ, :DynamicQuantitiesMakieExt).DQConversion
#
#lines(range(start=-1.0*DQ.u"mm", stop=1.0*DQ.u"mm", length=50), randn(50);
#    axis = (; dim1_conversion=DQConversion(DQ.us"mm")),
#)

display

@jusack
Copy link
Author

jusack commented May 13, 2025

That´s a good thought, I agree that we can probably reach the same level of convenience by using the unit already attached to the values of the axis as given by the user. So if the user gives values in km it will be km etc., for Unitful it would also be intuitive to just use uconvert (or |> u"mm" ) before plotting to achieve the desired unit. Otherwise the Makie.UnitfulConversion option is still there.

@ffreyer If you agree I would try to come up with something for that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Work in progress
Development

Successfully merging this pull request may close these issues.

3 participants