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

Converting between integer types #22235

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions base/math.jl
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,11 @@ end

"""
clamp(x, lo, hi)
clamp(x, T)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typically in Base APIs, the type argument comes first. So this should be clamp(T, x).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Documentation wasn't updated.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pushed a commit to fix that.


Return `x` if `lo <= x <= hi`. If `x < lo`, return `lo`. If `x > hi`, return `hi`. Arguments
are promoted to a common type.
are promoted to a common type. If the type `T` is given `lo` and `hi` are typemin(T)
and typemax(T) respectively and the result is converted to `T`

```jldoctest
julia> clamp.([pi, 1.0, big(10.)], 2., 9.)
Expand All @@ -59,10 +61,13 @@ clamp(x::X, lo::L, hi::H) where {X,L,H} =
convert(promote_type(X,L,H), lo),
convert(promote_type(X,L,H), x)))

clamp( x, ::Type{T} ) where {T} = clamp( x, typemin(T), typemax(T) ) % T
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No spaces around parens (here and elsewhere)

"""
clamp!(array::AbstractArray, lo, hi)
clamp!(array::AbstractArray, T)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line should be removed since you aren't defining this method


Restrict values in `array` to the specified range, in-place.
Restrict values in `array` to the specified range, in-place. If the type `T` is
given `lo` and `hi` are typemin(T) and typemax(T) respectively
See also [`clamp`](@ref).
"""
function clamp!(x::AbstractArray, lo, hi)
Expand All @@ -72,6 +77,13 @@ function clamp!(x::AbstractArray, lo, hi)
x
end

function clamp!(x::AbstractArray, ::Type{T}) where {T}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not strictly related, but is the clamp! function even necessary? Seems to me it could be written

A .= clamp.(lo, A, hi)

instead of

clamp!(A, lo, hi)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I completely agree. It seemed strange that the vector version of clamp has been deprecated but the in place version hadn't.

Copy link
Member

@KristofferC KristofferC Jun 6, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't that the case with multiple in place functions right now, e.g:

fill!(A, x) -> A .= x

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fill! is a little different IMO since it's intended to work on the array as a whole, whereas this clamp! method is more like implicit vectorization.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does "intended to work on the array as a whole" mean?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess I think of fill! as more like an initialization operation for the array (even though it isn't exactly)

@inbounds for i in eachindex(x)
x[i] = clamp(x[i], T )
end
x
end

# evaluate p[1] + x * (p[2] + x * (....)), i.e. a polynomial via Horner's rule
macro horner(x, p...)
ex = esc(p[end])
Expand Down
11 changes: 11 additions & 0 deletions test/math.jl
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,22 @@

@test clamp.([0, 1, 2, 3, 4], 1.0, 3.0) == [1.0, 1.0, 2.0, 3.0, 3.0]
@test clamp.([0 1; 2 3], 1.0, 3.0) == [1.0 1.0; 2.0 3.0]

@test clamp(-200,Int8) === typemin(Int8)
@test clamp(100,Int8) === Int8(100)
@test clamp(200,Int8) === typemax(Int8)

begin
x = [0.0, 1.0, 2.0, 3.0, 4.0]
clamp!(x, 1, 3)
@test x == [1.0, 1.0, 2.0, 3.0, 3.0]
end
begin
x = [-1000,0,1000]
clamp!(x, UInt8)
@test x == UInt8[0, 0, 255]
end

end

@testset "constants" begin
Expand Down