tryparse: parse string to Nullable #9487

tanmaykm · 2014-12-29T17:33:33Z

Introduces methods that parse a string as the indicated type and return a Nullable with the result instead of throwing exception:

maybeint{T<:Integer}(::Type{T<:Integer},s::AbstractString)
maybefloat32(s::AbstractString) and maybefloat64(s::AbstractString)

Ref: discussions at #9316, #3631, #5704

dhoegh · 2014-12-29T18:39:33Z

Just a thought why not maybe{T}(::Type{T},s::AbstractString) for all types, hence you collect the functions similar to zero.

tanmaykm · 2014-12-29T18:58:52Z

Yes, maybe will be nicer. Thanks for the suggestion.

nalimilan · 2014-12-29T21:38:48Z

Introducing the maybe terminology while the type is called Nullable is a bit weird. Also, for consistency with the current parseint, parsefloat, you could merge all of these functions into something like parse(::Type{Int}, ...), parse(::Type{Float}, ...), parse(::Type{Nullable{Int}}, ...), parse(::Type{Nullable{Float}}, ...), etc.

EDIT: fix types by adding ::Type{...}

ivarne · 2014-12-29T21:51:54Z

@nalimilan That seems like a massive improvement in API. Seems like Nullable will soon need some shorter name.

dhoegh · 2014-12-29T21:54:05Z

+1 for the API @nalimilan.

tanmaykm · 2014-12-30T03:35:54Z

@nalimilan parse is even better. Shall update the PR and manual, unless there are any other views.

Not sure why the AppVeyor build is failing though. Seems spurious, but it failed even after re-running the build.

nalimilan · 2014-12-30T07:37:27Z

Or to avoid the conflict in meanings with the current parse function, the new one could be named something like parseval. (It would also be a funny pun with a French mathematician's name...)

nalimilan · 2014-12-30T07:40:52Z

@tanmaykm The AppVeyor failure doesn't sound so spurious, as the failure happens in maybefloat64. But why would it happen only on Windows, I don't know.

tanmaykm · 2014-12-30T08:03:42Z

The AppVeyor logs report:

exception on 3: ERROR: test failed: (NaN == 64.0)
614 in expression: get(maybefloat64("\x034")) == 64.0

whereas strings.jl line 1315 is:

@test get(maybefloat64("64")) == 64.0

Seems like some sort of corruption.

vtjnash · 2015-01-02T00:39:47Z

sadly, that probably is due to #7906

vtjnash · 2015-03-10T12:49:20Z

bump. maybe we can merge this now?

toivoh · 2015-03-10T13:05:57Z

About api:
I'm not so sure about using eg

parse(Int, ...)

and

parse(Nullable{Int}, ...)

to make the distinction of whether a non-Int input should be allowed and returned add a null result. The latter call looks to like that it would look for a text representation of a (nullable) Int or null, and throw an error if neither was found.

tanmaykm · 2015-03-10T18:25:25Z

Will rebase and test again.

JeffBezanson · 2015-03-10T18:40:20Z

@toivoh good point. We could have parse and tryparse. Together they could replace parseint, parsefloat, float64_isvalid, and float32_isvalid, so still a net reduction of 2 functions. This PR should definitely remove the _isvalid functions.

JeffBezanson · 2015-03-10T18:43:25Z

base/string.jl

+    base = convert(T,base)
+    m::T = div(typemax(T)-base+1,base)
+    n::T = 0
+    while n <= m


We should avoid duplicating all of this code.

Yes, will be removing the duplication. Will pass the error string back through an optional ArgumentError parameter so that only methods that throw would incur the overhead of creating it.

tanmaykm · 2015-03-12T06:30:43Z

Help! Any clues as to why this build is failing? https://travis-ci.org/JuliaLang/julia/builds/54050279

nalimilan · 2015-03-12T08:54:52Z

Usually this kind of error means that a compile-time function call relies on a type that's defined in a file that's included later in the bootstrap process. Here, it looks like parsing the literal array at

julia/base/combinatorics.jl

Line 6 in 5b6468a

const _fact_table128 =

implies a call to parseint which involves BigInt in one way or another. But I have a hard time understanding why this would only happen on 32-bit, and how your changes may have triggered this.

tanmaykm · 2015-03-13T10:09:14Z

Thanks @nalimilan, I was able to reorder a portion of combinatorics.jl to fix the build issue. Not sure if the Appveyor issue is related.

tanmaykm · 2015-03-13T10:26:06Z

This now exports the tryparse method:

tryparse{T<:Integer}(::Type{T<:Integer},s::AbstractString)
tryparse(::Type{Float..},s::AbstractString)
a few variants of the above

And:

tryparse(Float.., ...) call the corresponding C functions jl_try_strtof, jl_try_substrtof, jl_try_strtod and jl_try_substrtod.
The parseint, parsefloat, float64_isvalid and float32_isvalid methods wrap the corresponding tryparse methods.
The jl_strtod, jl_strtof, ... functions are wrappers over the jl_try_str... functions.

This should fix #10498 as well.

I shall rebase and add documentation if this looks fine. Should the ..._isvalid methods be deprecated?

ViralBShah · 2015-03-13T11:04:12Z

Unless the ..._isvalid methods were exported, I don't think deprecation is necessary.

tkelman · 2015-03-13T11:19:02Z

float64_isvalid is used by JSON.jl, and presumably other packages as well. A deprecation would help lessen some breakage.

nalimilan · 2015-03-13T13:52:19Z

base/sysimg.jl

@@ -180,6 +180,27 @@ big(n::Integer) = convert(BigInt,n)
 big(x::FloatingPoint) = convert(BigFloat,x)
 big(q::Rational) = big(num(q))//big(den(q))

+const _fact_table128 =


I don't think you should put these lines here. This file contains almost exclusively includes. Have you tried simply moving include("combinatorics.jl") below the BigInt section?

There are cross dependencies that break if the entire file is shifted. These were the only UInt128 dependent code that was there. I thought of putting these in an int128funcs.jl file, but this seemed too small to go into a separate file.

We have to come up with something else. Having a factorial table in sysimg.jl is bananas. What depends on combinatorics.jl? I thought almost nothing would depend on this; it's all extra library functions that we've discussed removing from Base.

parsing UInt128 literals seems to require BigInt in 32 bit environment

Looks like gmp.jl depends on factorial and binomial. The simple generic definitions of those can be moved to intfuncs.jl. That should solve it.

I think it will be fine if I move the binomial and factorial from gmp.jl and mpfr.jl into combinatorics.jl.

Ok. So should they be moved to intfuncs.jl or combinatorics.jl since it already has the other variants?

nalimilan · 2015-03-13T13:58:59Z

I think I liked better parse(Nullable{Int}, ...) than tryparse(Int, ...). Especially since as @ivarne said above, with a new syntax for Nullable, we could imagine writing it as parse(Int?, ...). But well...

Are there any other cases where a function should come in two versions, one returning a Nullable? Just to check that the try* pattern generalizes well.

Introduces the tryparse method: - tryparse{T<:Integer}(::Type{T<:Integer},s::AbstractString) - tryparse(::Type{Float..},s::AbstractString) - a few variants of the above And: - tryparse(Float.., ...) call the corresponding C functions jl_try_strtof, jl_try_substrtof, jl_try_strtod and jl_try_substrtod. - The parseint, parsefloat, float64_isvalid and float32_isvalid methods wrap the corresponding tryparse methods. - The jl_strtod, jl_strtof, ... functions are wrappers over the jl_try_str... functions. This should fix JuliaLang#10498 as well. Ref: discussions at JuliaLang#9316, JuliaLang#3631, JuliaLang#5704

JeffBezanson · 2015-03-17T16:44:49Z

Could you move this branch to JuliaLang so I can work on it too?
I just moved a copy there, but I'll delete it and we can use yours instead.

JeffBezanson · 2015-03-17T16:52:12Z

@nalimilan Parsing numbers seems to be unusual in that (1) it is very performance-critical, (2) it has a failure mode that is exactly halfway to needing an exception. Half the time, you want an exception if the string is not a number, and half the time it just means you will handle the string differently. That's why I think parse/tryparse is justified.

tanmaykm · 2015-03-17T17:02:29Z

Ok. Moved to the tryparse branch on JuliaLang

JeffBezanson · 2015-03-17T17:17:30Z

replaced by #10543

vtjnash mentioned this pull request Jan 15, 2015

float conversion fails on certain substrings #9781

Closed

JeffBezanson reviewed Mar 10, 2015
View reviewed changes

tanmaykm force-pushed the maybefloat branch 9 times, most recently from 77a30d0 to 30665d6 Compare March 12, 2015 04:40

tanmaykm force-pushed the maybefloat branch 2 times, most recently from 2931312 to 9441d5d Compare March 13, 2015 04:54

tanmaykm changed the title ~~maybefloat & maybeint: parse string to Nullable~~ tryparse: parse string to Nullable Mar 13, 2015

nalimilan reviewed Mar 13, 2015
View reviewed changes

nalimilan mentioned this pull request Mar 16, 2015

Why is parseint(Int,x) not called parse(Int, x) #10531

Closed

tanmaykm force-pushed the maybefloat branch 2 times, most recently from 521fabe to a48582a Compare March 17, 2015 13:59

tanmaykm added 3 commits March 17, 2015 21:57

deprecate float64_isvalid, float32_isvalid

ede4fed

reshuffle combinatorics.jl

df239aa

tanmaykm force-pushed the maybefloat branch from a48582a to df239aa Compare March 17, 2015 16:38

JeffBezanson closed this Mar 17, 2015

JeffBezanson mentioned this pull request Mar 17, 2015

replace number parsing functions with parse and tryparse #10543

Merged

tanmaykm deleted the maybefloat branch March 18, 2015 01:45

tanmaykm mentioned this pull request Mar 18, 2015

correctly parse integer types in readdlm. fixes #9289 #10560

Merged

nalimilan mentioned this pull request Feb 7, 2016

getaddrinfo throws UVError "unknown node or service" #14972

Open

nalimilan mentioned this pull request Jun 17, 2016

POC/WIP/RFC: faster integer parsing; define parse semantics #16981

Closed

nalimilan mentioned this pull request Sep 10, 2020

Add default parameter to numerical tryparse #37170

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tryparse: parse string to Nullable #9487

tryparse: parse string to Nullable #9487

tanmaykm commented Dec 29, 2014

dhoegh commented Dec 29, 2014

tanmaykm commented Dec 29, 2014

nalimilan commented Dec 29, 2014

ivarne commented Dec 29, 2014

dhoegh commented Dec 29, 2014

tanmaykm commented Dec 30, 2014

nalimilan commented Dec 30, 2014

nalimilan commented Dec 30, 2014

tanmaykm commented Dec 30, 2014

vtjnash commented Jan 2, 2015

vtjnash commented Mar 10, 2015

toivoh commented Mar 10, 2015

tanmaykm commented Mar 10, 2015

JeffBezanson commented Mar 10, 2015

JeffBezanson Mar 10, 2015

tanmaykm Mar 10, 2015

tanmaykm commented Mar 12, 2015

nalimilan commented Mar 12, 2015

tanmaykm commented Mar 13, 2015

tanmaykm commented Mar 13, 2015

ViralBShah commented Mar 13, 2015

tkelman commented Mar 13, 2015

nalimilan Mar 13, 2015

tanmaykm Mar 13, 2015

JeffBezanson Mar 17, 2015

tanmaykm Mar 17, 2015

JeffBezanson Mar 17, 2015

tanmaykm Mar 17, 2015

tanmaykm Mar 17, 2015

nalimilan commented Mar 13, 2015

JeffBezanson commented Mar 17, 2015

JeffBezanson commented Mar 17, 2015

tanmaykm commented Mar 17, 2015

JeffBezanson commented Mar 17, 2015

tryparse: parse string to Nullable #9487

tryparse: parse string to Nullable #9487

Conversation

tanmaykm commented Dec 29, 2014

dhoegh commented Dec 29, 2014

tanmaykm commented Dec 29, 2014

nalimilan commented Dec 29, 2014

ivarne commented Dec 29, 2014

dhoegh commented Dec 29, 2014

tanmaykm commented Dec 30, 2014

nalimilan commented Dec 30, 2014

nalimilan commented Dec 30, 2014

tanmaykm commented Dec 30, 2014

vtjnash commented Jan 2, 2015

vtjnash commented Mar 10, 2015

toivoh commented Mar 10, 2015

tanmaykm commented Mar 10, 2015

JeffBezanson commented Mar 10, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tanmaykm commented Mar 12, 2015

nalimilan commented Mar 12, 2015

tanmaykm commented Mar 13, 2015

tanmaykm commented Mar 13, 2015

ViralBShah commented Mar 13, 2015

tkelman commented Mar 13, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nalimilan commented Mar 13, 2015

JeffBezanson commented Mar 17, 2015

JeffBezanson commented Mar 17, 2015

tanmaykm commented Mar 17, 2015

JeffBezanson commented Mar 17, 2015