-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
add parse(Complex{T}, s) #24713
add parse(Complex{T}, s) #24713
Conversation
@@ -229,3 +229,21 @@ end | |||
@test tryparse(Float32, "1.23") === Nullable(1.23f0) | |||
@test tryparse(Float16, "1.23") === Nullable(Float16(1.23)) | |||
|
|||
# parsing complex numbers (#22250) | |||
@testset "complex parsing" begin |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might also be good to add some tests for parsing bogus stuff, like 1+2ij
or 1im-3im
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
I'm not entirely sold on being able to parse things that aren't actual Julia values, e.g. |
@ararslan, my sense is that Sometimes this means parsing a subset of valid Julia expressions, e.g. In the case of parsing complex numbers, I would say that if the user writes |
Okay, now I'm sold. |
I updated the implementation to extend As a side effect, I was able to shorten some of the |
AppVeyor is failing with
which again seems unrelated? Travis OSX is failing with the unrelated error The unreliability of CI these days really makes PRs difficult; it feels like it's been months since I've seen a completely green CI. 😢 |
Yeah the CI situation nowadays is pretty dire. I believe Curtis Vogt has a fix in the works for the libgit2 stuff, which should fix the issues on Linux, Windows, and FreeBSD. I've been seeing the OpenBLAS issue on Travis macOS pretty consistently lately; I wonder if the version of GCC in Homebrew changed and now versions aren't matching properly or something. |
Pushed a slight change to NEWS; let's see if CI is better now. |
64-bit Windows and macOS failures are unrelated. |
Okay to merge, then? |
This fixes a string-indexing bug introduced in #24713 (Julia 0.7). Sometimes, this would cause `parse(Complex{T}, string)` to throw a `StringIndexError` rather than an `ArgumentError`, e.g. for `parse(ComplexF64, "3 β+ 4im")` or `parse(ComplexF64, "3 + 4αm")`. (As far as I can tell, it can never cause parsing to fail for valid strings.) The source of the error is that if `i` is the index of an ASCII character in a string `s`, then `s[i+1]` is valid (even if the next character is non-ASCII) but `s[i-1]` is invalid if the previous character is non-ASCII.
This fixes a string-indexing bug introduced in #24713 (Julia 0.7). Sometimes, this would cause `parse(Complex{T}, string)` to throw a `StringIndexError` rather than an `ArgumentError`, e.g. for `parse(ComplexF64, "3 β+ 4im")` or `parse(ComplexF64, "3 + 4αm")`. (As far as I can tell, it can never cause parsing to fail for valid strings.) The source of the error is that if `i` is the index of an ASCII character in a string `s`, then `s[i+1]` is valid (even if the next character is non-ASCII) but `s[i-1]` is invalid if the previous character is non-ASCII. (cherry picked from commit f71228d)
Closes #22250.
parse(Complex{T}, s::String)
now works, supportinga ± bIM
forIM
in (i
,j
,im
), with arbitrary whitespace.Closes #21935.
readdlm(io, ',', Complex{T})
now works.I also needed a fix for
parse(BigFloat, s)
, because it was failing ifs
had trailing whitespace (which is allowed by other number parsers in Julia). Added a test, and tightened up theBigFloat
parsing tests (which were using≈
when they could/should have been using==
).