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

isnumber("") return true #14156

Closed
songroom opened this issue Nov 26, 2015 · 16 comments · Fixed by #20342
Closed

isnumber("") return true #14156

songroom opened this issue Nov 26, 2015 · 16 comments · Fixed by #20342
Milestone

Comments

@songroom
Copy link

julia> isnumber("")
true

julia> versioninfo()
Julia Version 0.4.1
Commit cbe1bee* (2015-11-08 10:33 UTC)
Platform Info:
  System: Windows (x86_64-w64-mingw32)
  CPU: Intel(R) Xeon(R) CPU E5-2630 v2 @ 2.60GHz
  WORD_SIZE: 64
  BLAS: libopenblas (USE64BITINT DYNAMIC_ARCH NO_AFFINITY Sandybridge)
  LAPACK: libopenblas64_
  LIBM: libopenlibm
  LLVM: libLLVM-3.3

@yuyichao
Copy link
Contributor

This seems to be consistent with the doc.

Tests whether a character is numeric, or whether this is true for all elements of a string.

An empty string trivially satisfies this.

@nalimilan
Copy link
Member

One could equally well argue that false should be returned in that case. I'd rather raise an exception, especially given that "" is currently the only string for which isnumber is true which cannot be parsed as a (big) integer:

julia> parse(Int, "")
ERROR: ArgumentError: premature end of integer: ""

@yuyichao
Copy link
Contributor

It's consistent with other similar functions and I don't think it's a good idea to make some of them special.

@yuyichao
Copy link
Contributor

Also note that isnumber supports unicode Number category so "" is far from the only thing that can't be parsed as an integer.

julia> isnumber("¼")
true

julia> parse(Int, "¼")
ERROR: ArgumentError: invalid base 10 digit '¼' in "¼"
 [inlined code] from ./strings/io.jl:66
 in tryparse_internal at ./parse.jl:92
 in tryparse_internal at ./parse.jl:136
 in parse at ./parse.jl:146

@nalimilan
Copy link
Member

I must admit it's also consistent with all:

julia> all(Bool[])
true

Looks like this is a well-established principle: https://en.wikipedia.org/wiki/Vacuous_truth

But this gives rise to funny results like isupper("") & islower("") == true.

@JeffBezanson
Copy link
Member

I think it would be better not to implicitly lift these functions to strings, and write all(isnumber, str).

@yuyichao
Copy link
Contributor

Let's do that when jb/functions is ready.

@kshyatt
Copy link
Contributor

kshyatt commented Jan 25, 2017

jb/functions was ready, wasn't it? Can we make this change now?

@tkelman
Copy link
Contributor

tkelman commented Jan 26, 2017

yes though ideally it should be a deprecation so there's a limited window to get this into 0.6

@StefanKarpinski StefanKarpinski added this to the 0.6.0 milestone Jan 30, 2017
@StefanKarpinski
Copy link
Member

Let's go ahead and deprecate the auto-vectorised version here.

@StefanKarpinski
Copy link
Member

I also think this is a pretty surprising function since I would expect it to have something to do with the Number type and it doesn't at all – instead it has to do with string parsing.

@JeffBezanson
Copy link
Member

This also applies to all the other digit predicates, isprint, ispunct, islower, iscntrl, isalpha, etc. We can deprecate all of them, with the possible exception of isascii(string) which seems reasonable.

@nalimilan
Copy link
Member

PR #14387 merged all these functions into a single charprop function, to which one would pass the requested property type. I still think it's a good idea and I could rebase the relevant part if you like.

@StefanKarpinski
Copy link
Member

Yes please – let's go with that.

@JeffBezanson
Copy link
Member

That PR was pretty controversial, and also orthogonal to whether character property testing is lifted to strings.

@nalimilan
Copy link
Member

I don't think the contents of the PR were very controversial, except for the parts I debated in a long discussion with Scott (i.e. whether to use types or not). But you're right it's mostly orthogonal to this issue, I was just reacting to your statement "we can deprecate all of them".

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

Successfully merging a pull request may close this issue.

7 participants