-
-
Notifications
You must be signed in to change notification settings - Fork 344
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
base: master
Are you sure you want to change the base?
Conversation
I see that the reference images actually hit one case where the automatic unit is different now. How should I proceed here? |
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 |
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 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 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. |
Yea, I've also had mixed feelings about 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")),
#) |
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 @ffreyer If you agree I would try to come up with something for that |
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:
For a non-symmetric axis everything is fine
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:


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:
This PR:
Type of change
Delete options that do not apply: