Skip to content

Commit

Permalink
Close issue #78.
Browse files Browse the repository at this point in the history
  • Loading branch information
ajkeller34 committed May 11, 2017
1 parent 2916ba2 commit 2e8c2b3
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 23 deletions.
11 changes: 11 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,14 @@
- v0.2.3
- Dimensionful quantities are no longer accepted for `floor`, `ceil`, `trunc`, `round`,
`isinteger`. The choice of units can yield physically different results.

This comment has been minimized.

Copy link
@other-mickk

other-mickk May 13, 2017

What is a user who only needs 3 significant figures out of 204.27mm supposed to do?

This comment has been minimized.

Copy link
@ajkeller34

ajkeller34 May 14, 2017

Author Collaborator

Thanks for your interest in Unitful. If you want the old behavior, you can define and use methods like the following:

@inline round_numericpart(x::Unitful.Quantity) = round(Unitful.ustrip(x))*Unitful.unit(x)
@inline round_numericpart(x::Number) = round(x)

Ordinarily one expects operations to be invariant under a change of units, so I think the user should be explicit about wanting this behavior (at least, I've personally been bitten by this). You're welcome to open an issue to discuss further or argue for different behavior.

The functions are defined for dimensionless quantities, and return unitless numbers.
Closes [#78](https://github.com/ajkeller34/Unitful.jl/issues/78).
- Added `gn`, a constant quantity for the gravitational acceleration on earth
[#75](https://github.com/ajkeller34/Unitful.jl/pull/75).
- Added `ge`, the gravitational acceleration on earth as a unit
[#75](https://github.com/ajkeller34/Unitful.jl/pull/75).
- Added `lbf`, pounds-force unit
[#75](https://github.com/ajkeller34/Unitful.jl/pull/75).
- v0.2.2
- Fixed a bug in promotion involving `ContextUnits` where the promotion context might
not be properly retained.
Expand Down
13 changes: 7 additions & 6 deletions src/Unitful.jl
Original file line number Diff line number Diff line change
Expand Up @@ -789,9 +789,6 @@ end
abs(x::Quantity) = Quantity(abs(x.val), unit(x))
abs2(x::Quantity) = Quantity(abs2(x.val), unit(x)*unit(x))

trunc(x::Quantity) = Quantity(trunc(x.val), unit(x))
round(x::Quantity) = Quantity(round(x.val), unit(x))

copysign(x::Quantity, y::Number) = Quantity(copysign(x.val,y/unit(y)), unit(x))
flipsign(x::Quantity, y::Number) = Quantity(flipsign(x.val,y/unit(y)), unit(x))

Expand Down Expand Up @@ -872,15 +869,19 @@ end
==(x::Number, y::Quantity) = ==(y,x)
<=(x::Quantity, y::Quantity) = <(x,y) || x==y

for f in (:zero, :floor, :ceil)
@eval ($f)(x::Quantity) = Quantity(($f)(x.val), unit(x))
for f in (:floor, :ceil, :trunc, :round, :isinteger)
@eval ($f)(x::Quantity) = error("$($f) can only be well-defined for dimensionless ",
"numbers. For dimensionful numbers, different input units yield physically ",
"different results.")
@eval ($f)(x::DimensionlessQuantity) = ($f)(uconvert(NoUnits, x))
end

zero(x::Quantity) = Quantity(zero(x.val), unit(x))
zero{T,D,U}(x::Type{Quantity{T,D,U}}) = zero(T)*U()

one(x::Quantity) = one(x.val)
one{T,D,U}(x::Type{Quantity{T,D,U}}) = one(T)

isinteger(x::Quantity) = isinteger(x.val)
isreal(x::Quantity) = isreal(x.val)
isfinite(x::Quantity) = isfinite(x.val)
isinf(x::Quantity) = isinf(x.val)
Expand Down
30 changes: 13 additions & 17 deletions test/runtests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -520,8 +520,9 @@ end
@test inv([1 1; -1 1]u"nm") [0.5 -0.5; 0.5 0.5]u"nm^-1"
end
@testset "> Is functions" begin
@test isinteger(1.0m)
@test !isinteger(1.4m)
@test_throws ErrorException isinteger(1.0m)
@test isinteger(1.4m/mm)
@test !isinteger(1.4mm/m)
@test isfinite(1.0m)
@test !isfinite(Inf*m)
@test isnan(NaN*m)
Expand Down Expand Up @@ -771,14 +772,14 @@ end
end

@testset "Rounding" begin
@test @inferred(trunc(3.7m)) == 3.0m
@test trunc(-3.7m) == -3.0m
@test @inferred(floor(3.7m)) == 3.0m
@test floor(-3.7m) == -4.0m
@test @inferred(ceil(3.7m)) == 4.0m
@test ceil(-3.7m) == -3.0m
@test @inferred(round(3.7m)) == 4.0m
@test round(-3.7m) == -4.0m
@test_throws ErrorException floor(3.7m)
@test_throws ErrorException ceil(3.7m)
@test_throws ErrorException trunc(3.7m)
@test_throws ErrorException round(3.7m)
@test floor(1.0314m/mm) === 1031.0
@test ceil(1.0314m/mm) === 1032.0
@test trunc(-1.0314m/mm) === -1031.0
@test round(1.0314m/mm) === 1031.0
end

@testset "Sgn, abs, &c." begin
Expand Down Expand Up @@ -949,13 +950,8 @@ end
@test typeof(5m .* [1m, 2m, 3m]) == Array{typeof(1u"m^2"),1}
@test @inferred(eye(2)*V) == [1.0V 0.0V; 0.0V 1.0V]
@test @inferred(V*eye(2)) == [1.0V 0.0V; 0.0V 1.0V]
@static if VERSION >= v"0.6.0-dev.1632" # Make dot ops fusing, PR 17623

This comment has been minimized.

Copy link
@tkelman

tkelman May 11, 2017

this stopped being broken very recently, do you know exactly what from?

This comment has been minimized.

Copy link
@ajkeller34

ajkeller34 May 11, 2017

Author Collaborator

It was fine on April 5 and stopped being broken by April 12; if I search for "broadcast" in commits I find JuliaLang/julia#21331 which was merged on April 9. Probably this is what fixed it. How would you like me to proceed?

This comment has been minimized.

Copy link
@tkelman

tkelman May 11, 2017

at least in the range v"0.6.0-dev.1632" <= VERSION < v"0.6.0-pre.beta.85" it's likely still broken then, in which case it might be worth keeping the conditional in case it's ever necessary to bisect on other issues related to this package

This comment has been minimized.

Copy link
@ajkeller34

ajkeller34 May 11, 2017

Author Collaborator

I'll add this and retag.

@test_broken @inferred(eye(2).*V) == [1.0V 0.0V; 0.0V 1.0V]
@test_broken @inferred(V.*eye(2)) == [1.0V 0.0V; 0.0V 1.0V]
else
@test @inferred(eye(2).*V) == [1.0V 0.0V; 0.0V 1.0V]
@test @inferred(V.*eye(2)) == [1.0V 0.0V; 0.0V 1.0V]
end
@test @inferred(eye(2).*V) == [1.0V 0.0V; 0.0V 1.0V]
@test @inferred(V.*eye(2)) == [1.0V 0.0V; 0.0V 1.0V]
@test @inferred([1V 2V; 0V 3V].*2) == [2V 4V; 0V 6V]
@test @inferred([1V, 2V] .* [true, false]) == [1V, 0V]
@test @inferred([1.0m, 2.0m] ./ 3) == [1m/3, 2m/3]
Expand Down

0 comments on commit 2e8c2b3

Please sign in to comment.