Skip to content

Commit

Permalink
Attempt to allow preferunits to work with non-pure units (#478)
Browse files Browse the repository at this point in the history
* Attempt to allow preferunits to work with non-pure units

Should fix #457. Prior to this change, attempts to use something that
looked like "preferunits(C/ms)" would result in strange behavior, without
throwing any sort of errors. Now, that should work nicely.

* Warn user when preferunits causes redundant units

Issue a warning when preferunits creates redundant units, which could stop
it from sucessfully simplifying certain quantities.

* Fix new upreferred behavior for non-dimensional quantities

* Add tests for preferunits changes

* Update test/runtests.jl

Co-authored-by: Sebastian Stock <42280794+sostock@users.noreply.github.com>

* Fix preferunits tests

* Apply suggestions from code review

Co-authored-by: Mosè Giordano <giordano@users.noreply.github.com>

* Fix issues added by removing excess arrays

* No longer use string for key while checking for units of different scales

* Update test/runtests.jl

Co-authored-by: Sebastian Stock <42280794+sostock@users.noreply.github.com>

* Apply suggestions from code review

Co-authored-by: Sebastian Stock <42280794+sostock@users.noreply.github.com>

* Update runtests.jl

Co-authored-by: Sebastian Stock <42280794+sostock@users.noreply.github.com>
Co-authored-by: Mosè Giordano <giordano@users.noreply.github.com>
  • Loading branch information
3 people authored Nov 29, 2021
1 parent b17b1c5 commit bec43b6
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 4 deletions.
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.
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
13 changes: 12 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,16 @@ 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)
@test @inferred(upreferred(V/m)) == kg*m*C^-1*s^-2
@test dimension(upreferred(V/m)) == dimension(V/m)
# Reset preferred units to default, except for units of dimension 𝐋*𝐌*𝐈^-1*𝐓^-3,
# because upreferred has already been called for that dimension
Unitful.preferunits(A)

# 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

0 comments on commit bec43b6

Please sign in to comment.