Skip to content
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

Wrong units #4063

Closed
jariji opened this issue Jul 29, 2024 · 9 comments · Fixed by #4583
Closed

Wrong units #4063

jariji opened this issue Jul 29, 2024 · 9 comments · Fixed by #4583
Labels
bug Makie Backend independent issues (Makie core) units aka dim converts

Comments

@jariji
Copy link
Contributor

jariji commented Jul 29, 2024

  [13f3f980] CairoMakie v0.12.5
  [1986cc42] Unitful v1.21.0


julia> scatter(rand(5)u"m/s", rand(5)u"m/s")

The units are wrong.

image

@jariji jariji added the bug label Jul 29, 2024
@asinghvi17 asinghvi17 added units aka dim converts Makie Backend independent issues (Makie core) labels Jul 30, 2024
@SimonDanisch
Copy link
Member

SimonDanisch commented Jul 30, 2024

Why it switches to decimeter is because the default is to automatically switch to the best unit to represent the number range.
Why it drops the second is not really clear to me...
You could have a look at: https://github.com/MakieOrg/Makie.jl/blob/master/src/dim-converts/unitful-integration.jl
And see where it goes wrong!

@ffreyer
Copy link
Collaborator

ffreyer commented Jul 30, 2024

It's one of the base_unit() calls.

base_unit(q::Quantity) = base_unit(typeof(q))
base_unit(::Type{Quantity{NumT, DimT, U}}) where {NumT, DimT, U} = base_unit(U)
base_unit(::Type{Unitful.FreeUnits{U, DimT, nothing}}) where {DimT, U} = U[1]

It goes through these 3 in order where in the last one U = (m, s^-1), so that indexing just grabs m

@ffreyer
Copy link
Collaborator

ffreyer commented Sep 27, 2024

It looks like most of this is for adjusting unit prefixes to the range of numbers given. E.g. changing 10000m .. 54000m to 10km .. 54km. I wonder if we should just drop those conversions.
It seems difficult to apply them to compound units - e.g. if you have "A * B / C", should A, B or C get a prefix? Some people may also supply specific prefixes on purpose because they are commonly used (e.g. g/cm^3). It also looks like this is not generic atm, i.e. we'd have to define valid prefixes for every unit and add special cases to avoid things like dft (deci-feet).

@jariji
Copy link
Contributor Author

jariji commented Sep 27, 2024

Yeah keeping the user's units seems good.

@abhro
Copy link
Contributor

abhro commented Sep 28, 2024

Is there an option to do it at the top (or right) of the axes? I mean in the style that MATLAB does power of 10 representations (linear plot, scaled), then putting the units on top of the axis might help (see first image of https://tutorial45.com/matlab-plot-colors-and-styles/)

@aplavin
Copy link
Contributor

aplavin commented Sep 28, 2024

Agree, it would be both more familiar to many readers and less cluttered to show units once per axes – #3890.

@nathanrboyer
Copy link

There are no circumstances under which I want to plot deka-feet. 😑
Is there a quick switch to get ft back, and can d, c, da, and h be excluded from the automatic conversions?

Image

@ffreyer
Copy link
Collaborator

ffreyer commented Mar 12, 2025

You can turn off automatic conversions if you create the conversion manually. It's explained in ?Makie.UnitfulConversion

@nathanrboyer
Copy link

Okay, thanks. I got that working. I still think conversions from 10^-2 through 10^2 should be automatically avoided though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Makie Backend independent issues (Makie core) units aka dim converts
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants