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

replace number parsing functions with parse and tryparse #10543

Merged
merged 7 commits into from
Mar 17, 2015
Merged
Show file tree
Hide file tree
Changes from all 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
13 changes: 12 additions & 1 deletion NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -282,11 +282,15 @@ Deprecated or removed
names have been removed. To convert a scalar, use the type name, e.g.
`Int32(x)`. To convert an array to a different element type, use
`Array{T}(x)`, `map(T,x)`, or `round(T,x)`. To parse a string as an integer
or floating-point number, use `parseint` or `parsefloat` ([#1470], [#6211]).
or floating-point number, use `parse` ([#1470], [#6211]).

* Low-level functions from the C library and dynamic linker have been moved to
modules `Libc` and `Libdl`, respectively ([#10328]).

* The functions `parseint`, `parsefloat`, `float32_isvalid`, and `float64_isvalid`
have been replaced by `parse` and `tryparse` with a type argument
([#3631], [#5704], [#9487], [#10543]).

Julia v0.3.0 Release Notes
==========================

Expand Down Expand Up @@ -1070,6 +1074,7 @@ Too numerous to mention.
[#3483]: https://github.com/JuliaLang/julia/issues/3483
[#3523]: https://github.com/JuliaLang/julia/issues/3523
[#3605]: https://github.com/JuliaLang/julia/issues/3605
[#3631]: https://github.com/JuliaLang/julia/issues/3631
[#3649]: https://github.com/JuliaLang/julia/issues/3649
[#3665]: https://github.com/JuliaLang/julia/issues/3665
[#3688]: https://github.com/JuliaLang/julia/issues/3688
Expand Down Expand Up @@ -1158,6 +1163,7 @@ Too numerous to mention.
[#5671]: https://github.com/JuliaLang/julia/issues/5671
[#5677]: https://github.com/JuliaLang/julia/issues/5677
[#5703]: https://github.com/JuliaLang/julia/issues/5703
[#5704]: https://github.com/JuliaLang/julia/issues/5704
[#5726]: https://github.com/JuliaLang/julia/issues/5726
[#5737]: https://github.com/JuliaLang/julia/issues/5737
[#5748]: https://github.com/JuliaLang/julia/issues/5748
Expand Down Expand Up @@ -1278,6 +1284,7 @@ Too numerous to mention.
[#9425]: https://github.com/JuliaLang/julia/issues/9425
[#9434]: https://github.com/JuliaLang/julia/issues/9434
[#9452]: https://github.com/JuliaLang/julia/issues/9452
[#9487]: https://github.com/JuliaLang/julia/issues/9487
[#9569]: https://github.com/JuliaLang/julia/issues/9569
[#9575]: https://github.com/JuliaLang/julia/issues/9575
[#9578]: https://github.com/JuliaLang/julia/issues/9578
Expand All @@ -1295,6 +1302,10 @@ Too numerous to mention.
[#10150]: https://github.com/JuliaLang/julia/issues/10150
[#10180]: https://github.com/JuliaLang/julia/issues/10180
[#10228]: https://github.com/JuliaLang/julia/issues/10228
[#10328]: https://github.com/JuliaLang/julia/issues/10328
[#10332]: https://github.com/JuliaLang/julia/issues/10332
[#10333]: https://github.com/JuliaLang/julia/issues/10333
[#10400]: https://github.com/JuliaLang/julia/issues/10400
[#10446]: https://github.com/JuliaLang/julia/issues/10446
[#10458]: https://github.com/JuliaLang/julia/issues/10458
[#10543]: https://github.com/JuliaLang/julia/issues/10543
2 changes: 1 addition & 1 deletion base/Terminals.jl
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ let s = zeros(Int32, 2)
function Base.size(t::TTYTerminal)
@windows_only if ispty(t.out_stream)
try
h,w = map(parseint,split(readall(open(`stty size`, "r", t.out_stream)[1])))
h,w = map(x->parse(Int,x),split(readall(open(`stty size`, "r", t.out_stream)[1])))
w > 0 || (w = 80)
h > 0 || (h = 24)
return h,w
Expand Down
1 change: 0 additions & 1 deletion base/base.jl
Original file line number Diff line number Diff line change
Expand Up @@ -277,4 +277,3 @@ immutable Nullable{T}
Nullable() = new(true)
Nullable(value::T) = new(false, value)
end

4 changes: 2 additions & 2 deletions base/client.jl
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ function init_bind_addr()
bind_to = split(bytestring(opts.bindto), ":")
bind_addr = string(parseip(bind_to[1]))
if length(bind_to) > 1
bind_port = parseint(bind_to[2])
bind_port = parse(Int,bind_to[2])
else
bind_port = 0
end
Expand Down Expand Up @@ -358,7 +358,7 @@ function load_machine_file(path::AbstractString)
for line in split(readall(path),'\n'; keep=false)
s = map!(strip, split(line,'*'; keep=false))
if length(s) > 1
cnt = isnumber(s[1]) ? parseint(s[1]) : symbol(s[1])
cnt = isnumber(s[1]) ? parse(Int,s[1]) : symbol(s[1])
push!(machines,(s[2], cnt))
else
push!(machines,line)
Expand Down
40 changes: 0 additions & 40 deletions base/combinatorics.jl
Original file line number Diff line number Diff line change
Expand Up @@ -49,18 +49,6 @@ function gamma(n::Union(Int8,UInt8,Int16,UInt16,Int32,UInt32,Int64,UInt64))
@inbounds return Float64(_fact_table64[n-1])
end

function factorial(n::Integer)
n < 0 && throw(DomainError())
local f::typeof(n*n), i::typeof(n*n)
f = 1
for i = 2:n
f *= i
end
return f
end

factorial(x::Number) = gamma(x + 1) # fallback for x not Integer

# computes n!/k!
function factorial{T<:Integer}(n::T, k::T)
if k < 0 || n < 0 || k > n
Expand All @@ -75,34 +63,6 @@ function factorial{T<:Integer}(n::T, k::T)
end
factorial(n::Integer, k::Integer) = factorial(promote(n, k)...)

function binomial{T<:Integer}(n::T, k::T)
k < 0 && return zero(T)
sgn = one(T)
if n < 0
n = -n + k -1
if isodd(k)
sgn = -sgn
end
end
k > n && return zero(T)
(k == 0 || k == n) && return sgn
k == 1 && return sgn*n
if k > (n>>1)
k = (n - k)
end
x::T = nn = n - k + 1
nn += 1
rr = 2
while rr <= k
xt = div(widemul(x, nn), rr)
x = xt
x == xt || throw(OverflowError())
rr += 1
nn += 1
end
convert(T, copysign(x, sgn))
end

## other ordering related functions ##
function nthperm!(a::AbstractVector, k::Integer)
k -= 1 # make k 1-indexed
Expand Down
2 changes: 1 addition & 1 deletion base/dates/io.jl
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ function slotparse(slot::Slot{Month},x)
return Month(MONTHTOVALUE[slot.locale][lowercase(x)])
end
end
slotparse(slot::Slot{Millisecond},x) = !ismatch(r"[^0-9\s]",x) ? slot.period(parsefloat("."*x)*1000.0) : throw(SLOTERROR)
slotparse(slot::Slot{Millisecond},x) = !ismatch(r"[^0-9\s]",x) ? slot.period(Base.parse(Float64,"."*x)*1000.0) : throw(SLOTERROR)
slotparse(slot::Slot{DayOfWeekSlot},x) = nothing

function getslot(x,slot::DelimitedSlot,df,cursor)
Expand Down
2 changes: 1 addition & 1 deletion base/dates/periods.jl
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ for p in (:Year,:Month,:Week,:Day,:Hour,:Minute,:Second,:Millisecond)
# periodisless
@eval periodisless(x::$p,y::$p) = value(x) < value(y)
# AbstractString parsing (mainly for IO code)
@eval $p(x::AbstractString) = $p(parseint(Int64,x))
@eval $p(x::AbstractString) = $p(Base.parse(Int64,x))
# Period accessors
@eval $p(x::TimeType) = $p($(symbol(lowercase(string(p))))(x))
end
Expand Down
24 changes: 24 additions & 0 deletions base/deprecated.jl
Original file line number Diff line number Diff line change
Expand Up @@ -496,3 +496,27 @@ function to_index{T<:Real}(A::AbstractArray{T})
depwarn("indexing with non Integer AbstractArrays is deprecated", :to_index)
Int[to_index_nodep(x) for x in A]
end

function float_isvalid{T<:Union(Float32,Float64)}(s::AbstractString, out::Array{T,1})
tf = tryparse(T, s)
isnull(tf) || (out[1] = get(tf))
!isnull(tf)
end
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Why no depwarn here?

A comment refering to #10543 would also be great so that it is easier to see where a deprecation is comming from (without digging into git history).

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

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

This is actually just a utility used by actual deprecated functions. There never was a float_isvalid before.

Copy link

Choose a reason for hiding this comment

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

Unrelated question, in this function isnull is called twice, would it in
general offer a performance gain to save the value from the first call in a
local variable or is the execution time equivalent to that of fetching the
variable? Is there a definable turning point in function complexity at
which saving its output becomes beneficial? Can saving array values as
local variables offer improvement or does Julia automatically detect and
optimize reusable local values?
On Mar 17, 2015 8:01 PM, "Jeff Bezanson" [email protected] wrote:

In base/deprecated.jl
#10543 (comment):

@@ -496,3 +496,19 @@ function to_index{T<:Real}(A::AbstractArray{T})
depwarn("indexing with non Integer AbstractArrays is deprecated", :to_index)
Int[to_index_nodep(x) for x in A]
end
+
+function float_isvalid{T<:Union(Float32,Float64)}(s::AbstractString, out::Array{T,1})

  • tf = tryparse(T, s)
  • isnull(tf) || (out[1] = get(tf))
  • !isnull(tf)
    +end

This is actually just a utility used by actual deprecated functions. There
never was a float_isvalid before.


Reply to this email directly or view it on GitHub
https://github.com/JuliaLang/julia/pull/10543/files#r26598614.

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

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

Yes, in general re-using a result is faster than calling a function twice.
However in this case, isnull is so trivial:

isnull(x::Nullable) = x.isnull

that it will be inlined and I'm pretty sure there'd be nothing to gain by saving its result.

In a very small number of cases Julia can automatically avoid redundant work from calling a function multiple times with the same arguments. However this is hard to do in general. For example Arrays are mutable, so it's meaningful to allocate two different ones with the same contents --- you might be planning to mutate them differently. So we can't do this optimization in such cases. Much more can be said but this topic gets complicated quickly.


function float32_isvalid(s::AbstractString, out::Array{Float32,1})
depwarn("float32_isvalid is deprecated, use tryparse(Float32,s) instead", :float32_isvalid)
float_isvalid(s, out)
end

function float64_isvalid(s::AbstractString, out::Array{Float64,1})
depwarn("float64_isvalid is deprecated, use tryparse(Float64,s) instead", :float64_isvalid)
float_isvalid(s, out)
end

@deprecate parsefloat(s::AbstractString) parse(Float64,s)
@deprecate parsefloat(T, s) parse(T, s)

@deprecate parseint(s) parse(Int, s)
@deprecate parseint(s,base) parse(Int, s, base)
@deprecate parseint(T::Type, s) parse(T, s)
@deprecate parseint(T::Type, s, base) parse(T, s, base)
4 changes: 2 additions & 2 deletions base/env.jl
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,6 @@ function tty_size()
return size(os)
end
end
return (parseint(get(ENV,"LINES","24")),
parseint(get(ENV,"COLUMNS","80")))
return (parse(Int,get(ENV,"LINES","24")),
parse(Int,get(ENV,"COLUMNS","80")))
end
5 changes: 1 addition & 4 deletions base/exports.jl
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,7 @@ export
fldmod,
flipsign,
float,
tryparse,
floor,
fma,
frexp,
Expand Down Expand Up @@ -806,8 +807,6 @@ export
eachmatch,
endswith,
escape_string,
float32_isvalid,
float64_isvalid,
graphemes,
hex,
hex2bytes,
Expand Down Expand Up @@ -844,8 +843,6 @@ export
nextind,
normalize_string,
oct,
parsefloat,
parseint,
prevind,
print,
print_escaped,
Expand Down
4 changes: 2 additions & 2 deletions base/floatfuncs.jl
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@ num2hex(x::Float64) = hex(box(UInt64,unbox(Float64,x)),16)

function hex2num(s::AbstractString)
if length(s) <= 8
return box(Float32,unbox(Int32,parseint(Int32,s,16)))
return box(Float32,unbox(Int32,parse(Int32,s,16)))
end
return box(Float64,unbox(Int64,parseint(Int64,s,16)))
return box(Float64,unbox(Int64,parse(Int64,s,16)))
end

@vectorize_1arg Number abs
Expand Down
20 changes: 14 additions & 6 deletions base/gmp.jl
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ export BigInt
import Base: *, +, -, /, <, <<, >>, >>>, <=, ==, >, >=, ^, (~), (&), (|), ($),
binomial, cmp, convert, div, divrem, factorial, fld, gcd, gcdx, lcm, mod,
ndigits, promote_rule, rem, show, isqrt, string, isprime, powermod,
sum, trailing_zeros, trailing_ones, count_ones, base, parseint,
sum, trailing_zeros, trailing_ones, count_ones, base, tryparse_internal,
serialize, deserialize, bin, oct, dec, hex, isequal, invmod,
prevpow2, nextpow2, ndigits0z, widen, signed

Expand Down Expand Up @@ -74,17 +74,25 @@ widen(::Type{BigInt}) = BigInt
signed(x::BigInt) = x

BigInt(x::BigInt) = x
BigInt(s::AbstractString) = parseint(BigInt,s)
BigInt(s::AbstractString) = parse(BigInt,s)

function Base.parseint_nocheck(::Type{BigInt}, s::AbstractString, base::Int)
function tryparse_internal(::Type{BigInt}, s::AbstractString, base::Int, raise::Bool)
_n = Nullable{BigInt}()
s = bytestring(s)
sgn, base, i = Base.parseint_preamble(true,s,base)
if i == 0
raise && throw(ArgumentError("premature end of integer: $(repr(s))"))
return _n
end
z = BigInt()
err = ccall((:__gmpz_set_str, :libgmp),
Int32, (Ptr{BigInt}, Ptr{UInt8}, Int32),
&z, SubString(s,i), base)
err == 0 || throw(ArgumentError("invalid BigInt: $(repr(s))"))
return sgn < 0 ? -z : z
if err != 0
raise && throw(ArgumentError("invalid BigInt: $(repr(s))"))
return _n
end
Nullable(sgn < 0 ? -z : z)
end

function BigInt(x::Union(Clong,Int32))
Expand Down Expand Up @@ -217,7 +225,7 @@ function serialize(s, n::BigInt)
serialize(s, base(62,n))
end

deserialize(s, ::Type{BigInt}) = Base.parseint_nocheck(BigInt, deserialize(s), 62)
deserialize(s, ::Type{BigInt}) = get(tryparse_internal(BigInt, deserialize(s), 62, true))

# Binary ops
for (fJ, fC) in ((:+, :add), (:-,:sub), (:*, :mul),
Expand Down
4 changes: 2 additions & 2 deletions base/int.jl
Original file line number Diff line number Diff line change
Expand Up @@ -248,15 +248,15 @@ floor{T<:Integer}(::Type{T},x::Integer) = convert(T,x)

macro int128_str(x)
if isa(x,AbstractString)
parseint(Int128,x)
parse(Int128,x)
else
Int128(x)
end
end

macro uint128_str(x)
if isa(x,AbstractString)
parseint(UInt128,x)
parse(UInt128,x)
else
UInt128(x)
end
Expand Down
38 changes: 38 additions & 0 deletions base/intfuncs.jl
Original file line number Diff line number Diff line change
Expand Up @@ -333,3 +333,41 @@ function isqrt(x::Union(Int64,UInt64,Int128,UInt128))
s = (s + div(x,s)) >> 1
s*s > x ? s-1 : s
end

function factorial(n::Integer)
n < 0 && throw(DomainError())
local f::typeof(n*n), i::typeof(n*n)
f = 1
for i = 2:n
f *= i
end
return f
end

function binomial{T<:Integer}(n::T, k::T)
k < 0 && return zero(T)
sgn = one(T)
if n < 0
n = -n + k -1
if isodd(k)
sgn = -sgn
end
end
k > n && return zero(T)
(k == 0 || k == n) && return sgn
k == 1 && return sgn*n
if k > (n>>1)
k = (n - k)
end
x::T = nn = n - k + 1
nn += 1
rr = 2
while rr <= k
xt = div(widemul(x, nn), rr)
x = xt
x == xt || throw(OverflowError())
rr += 1
nn += 1
end
convert(T, copysign(x, sgn))
end
4 changes: 2 additions & 2 deletions base/latex_symbols.jl
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ for c in child_nodes(root(xdoc))
if latex != nothing
L = strip(content(latex))
id = attribute(ce, "id")
U = string(map(s -> Char(parseint(s, 16)),
U = string(map(s -> Char(parse(Int, s, 16)),
split(id[2:end], "-"))...)
if ismatch(r"^\\[A-Za-z]+$",L) && !isa(U,ASCIIString)
if L in Ls
Expand Down Expand Up @@ -51,7 +51,7 @@ open("unicode-math-table.tex") do f
for L in eachline(f)
x = map(s -> rstrip(s, [' ','\t','\n']),
split(replace(L, r"[{}\"]+", "\t"), "\t"))
c = Char(parseint(x[2], 16))
c = Char(parse(Int, x[2], 16))
if (Base.is_id_char(c) || Base.isoperator(symbol(c))) &&
string(c) ∉ latex_strings && !isascii(c)
println(" \"", escape_string(x[3]), "\" => \"",
Expand Down
2 changes: 1 addition & 1 deletion base/multi.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1024,7 +1024,7 @@ end
function parse_connection_info(str)
m = match(r"^julia_worker:(\d+)#(.*)", str)
if m != nothing
(m.captures[2], parseint(Int16, m.captures[1]))
(m.captures[2], parse(Int16, m.captures[1]))
else
("", Int16(-1))
end
Expand Down
4 changes: 2 additions & 2 deletions base/nullable.jl
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ convert( ::Type{Nullable }, ::Void) = Nullable{Union()}()

function show{T}(io::IO, x::Nullable{T})
if x.isnull
@printf(io, "Nullable{%s}()", repr(T))
print(io, "Nullable{"); show(io, T); print(io, "}()")
else
@printf(io, "Nullable(%s)", repr(x.value))
print(io, "Nullable("); show(io, x.value); print(io, ")")
end
end

Expand Down
2 changes: 2 additions & 0 deletions base/number.jl
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,5 @@ zero(x::Number) = oftype(x,0)
zero{T<:Number}(::Type{T}) = convert(T,0)
one(x::Number) = oftype(x,1)
one{T<:Number}(::Type{T}) = convert(T,1)

factorial(x::Number) = gamma(x + 1) # fallback for x not Integer
2 changes: 1 addition & 1 deletion base/pkg/entry.jl
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,7 @@ function publish(branch::AbstractString)
Git.branch(dir="METADATA") == branch ||
error("METADATA must be on $branch to publish changes")
Git.run(`fetch -q`, dir="METADATA")
ahead_remote, ahead_local = map(parseint,split(Git.readchomp(`rev-list --count --left-right origin/$branch...$branch`, dir="METADATA"),'\t'))
ahead_remote, ahead_local = map(x->parse(Int,x),split(Git.readchomp(`rev-list --count --left-right origin/$branch...$branch`, dir="METADATA"),'\t'))
ahead_remote > 0 && error("METADATA is behind origin/$branch – run `Pkg.update()` before publishing")
ahead_local == 0 && error("There are no METADATA changes to publish")

Expand Down
Loading