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

stop supporting bases > 36 without explicit alphabet? #26571

Closed
StefanKarpinski opened this issue Mar 22, 2018 · 4 comments
Closed

stop supporting bases > 36 without explicit alphabet? #26571

StefanKarpinski opened this issue Mar 22, 2018 · 4 comments
Milestone

Comments

@StefanKarpinski
Copy link
Sponsor Member

https://discourse.julialang.org/t/what-is-the-effect-of-parse-on-char-types/9874/2

The fact that we have this seems like asking for trouble:

julia> parse(Int, 'a', base=36)
10

julia> parse(Int, 'A', base=36)
10

julia> parse(Int, 'a', base=37)
36

julia> parse(Int, 'A', base=37)
10

Perhaps it would be best to stop supporting bases larger than 36 since it's standard to parse hex in a case insensitive manner. At the very least we should make sure that we audit all the places where we print and parse bases larger than 36 and make sure that they're all internally consistent at least.

@JeffBezanson JeffBezanson added the status:triage This should be discussed on a triage call label Mar 22, 2018
@JeffBezanson
Copy link
Sponsor Member

Actually this might be the real issue:

julia> parse(Int, "z")
ERROR: ArgumentError: invalid base 10 digit 'z' in "z"
Stacktrace:
 [1] tryparse_internal(::Type{Int64}, ::String, ::Int64, ::Int64, ::Int64, ::Bool) at ./parse.jl:117
 [2] #parse#360(::Nothing, ::Function, ::Type{Int64}, ::String) at ./parse.jl:216
 [3] parse(::Type{Int64}, ::String) at ./parse.jl:216
 [4] top-level scope

julia> parse(Int, 'z')
35

We use 36 as the default base for parsing Chars, and 10 elsewhere. I think we should change the default to 10 everywhere.

@StefanKarpinski StefanKarpinski added this to the 1.0 milestone Mar 22, 2018
@StefanKarpinski StefanKarpinski removed the status:triage This should be discussed on a triage call label Mar 22, 2018
@ttparker
Copy link

The method parse(type, c::Char) is currently undocumented - should we add it to the documentation?

@o314
Copy link
Contributor

o314 commented Mar 23, 2018

Even in base < 36, there is not always a strictly direct 1 to 1 mapping to a digits + hexa base.

Ex: base32 crockford = base36 - I, L, O, U
Very useful to avoid confusion for human readable identifier (no more is it a 1, a L, or an I; a O or an 0 ?)

https://github.com/JuliaLang/julia/blob/master/base/parse.jl#L36-L44 seems a bit too much hardcoded

@ttparker
Copy link

@JeffBezanson It seems that there's still no docstring for the parse(T, ::Char) method? (I could try adding one myself, but I haven't contributed to Julia yet so it might take me a while to figure out how.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants