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

Attempt to allow preferunits to work with non-pure units #478

Merged
merged 13 commits into from
Nov 29, 2021
Merged
2 changes: 1 addition & 1 deletion src/Unitful.jl
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ end
const basefactors = _basefactors(Unitful)

include("types.jl")
const promotion = Dict{Symbol,Unit}()
const promotion = Dict{Symbol,FreeUnits}()

include("user.jl")
include("utils.jl")
Expand Down
2 changes: 1 addition & 1 deletion src/units.jl
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,6 @@ factory defaults, this function will return a product of powers of base SI units
(as [`Unitful.FreeUnits`](@ref)).
"""
@generated function upreferred(x::Dimensions{D}) where {D}
u = *(FreeUnits{((Unitful.promotion[name(z)]^z.power for z in D)...,),()}())
u = prod((NoUnits, (promotion[name(z)]^z.power for z in D)...))
:($u)
end
23 changes: 22 additions & 1 deletion src/user.jl
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,28 @@ function preferunits(u0::Units, u::Units...)
"For instance, it should not be used with units of dimension 𝐋^2.")
end
y = typeof(dim).parameters[1][1]
promotion[name(y)] = typeof(unit).parameters[1][1]
promotion[name(y)] = unit
end

# Let's run a quick check for anything where, for instance, current was defined is C/ms and time
# was defined in ns. We'll give a warning if this is the case so the user knows they may get
# unexpected (though still correct) results. There may be cases where people would want to do this,
# so we won't stop them, just let them know they're doing it.
sostock marked this conversation as resolved.
Show resolved Hide resolved
ulist = (typeof(i[2]).parameters[1] for i in promotion)
check = Dict{Dimensions, Unit}()
for a in ulist
ulistA = (i^(1/i.power) for i in a)
for i in ulistA
k = dimension(i)
if haskey(check, k)
if i!=check[k]
@warn "Preferred units contain complex units based on units of the same dimension but different scales: "*string(i)*" and "*string(check[k])*
"\nThis may be intentional, but otherwise could lead to redundant units, such as ms/s or kg/g"
end
else
check[k] = i
end
end
end

nothing
Expand Down
12 changes: 11 additions & 1 deletion test/runtests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ import Unitful:
J, A, N, mol, V,
mW, W,
dB, dB_rp, dB_p, dBm, dBV, dBSPL, Decibel,
Np, Np_rp, Np_p, Neper
Np, Np_rp, Np_p, Neper,
C

import Unitful: 𝐋, 𝐓, 𝐍, 𝚯

Expand Down Expand Up @@ -320,6 +321,15 @@ Unitful.uconvert(U::Unitful.Units, q::QQQ) = uconvert(U, Quantity(q.val, cm))

@testset "Promotion" begin
@testset "> Unit preferences" begin
# Should warn on possible redundant units issue (ms and s)
@test_logs (:warn, r"^Preferred units contain complex units") Unitful.preferunits(C/ms)
# Test for wacky prefered units functionality
Unitful.preferunits(C/s)
lukebemish marked this conversation as resolved.
Show resolved Hide resolved
@test @inferred(upreferred(V/m)) == kg*m*C^-1*s^-2
@test dimension(upreferred(V/m)) == dimension(V/m)
# Reset prefered units to default
Unitful.preferunits(A)
lukebemish marked this conversation as resolved.
Show resolved Hide resolved

# Only because we favor SI, we have the following:
@test @inferred(upreferred(N)) === kg*m/s^2
@test @inferred(upreferred(dimension(N))) === kg*m/s^2
Expand Down