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

strconv: add ParseComplex and FormatComplex #36815

Closed
wants to merge 48 commits into from

Conversation

pjebs
Copy link
Contributor

@pjebs pjebs commented Jan 27, 2020

Adds two functions to deal with complex numbers:

  • FormatComplex
  • ParseComplex

ParseComplex accepts complex numbers in this format: N+Ni

Fixes #36771

@googlebot googlebot added the cla: yes Used by googlebot to label PRs as having a valid CLA. The text of this label should not change. label Jan 27, 2020
@gopherbot
Copy link
Contributor

This PR (HEAD: 8afa9a5) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/216617 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Ian Lance Taylor:

Patch Set 1: Code-Review-2

Temporary -2 since the proposal has not been accepted.


Please don’t reply on this GitHub thread. Visit golang.org/cl/216617.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

This PR (HEAD: 8df2934) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/216617 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

This PR (HEAD: bdf80fd) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/216617 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

This PR (HEAD: 0f31e5a) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/216617 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Ian Lance Taylor:

Patch Set 4:

(2 comments)


Please don’t reply on this GitHub thread. Visit golang.org/cl/216617.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

This PR (HEAD: 04ebaf1) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/216617 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@pjebs pjebs changed the title strconv: Add ParseCompex function strconv: Add ParseComplex function Mar 28, 2020
@gopherbot
Copy link
Contributor

This PR (HEAD: 07a8ba4) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/216617 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

This PR (HEAD: cefda0c) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/216617 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Emmanuel Odeke:

Patch Set 8:

(26 comments)

Thank you Pj for the change. I've added some suggestions.

Please ensure that we can accept all combinations of:
REAL SIGN IMAG // IMAG SIGN REAL

I've started a simple format that just accepts for starters
a prototype for ParseComplex at:
https://play.golang.org/p/sK3o9iBNJ55
and that parses the basic form of:
REAL(PLAIN_FLOAT) SIGN IMAG(PLAIN_FLOAT) and
its various combinations. Please take a look an see if it sparks
some ideas.


Please don’t reply on this GitHub thread. Visit golang.org/cl/216617.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Emmanuel Odeke:

Patch Set 8:

(3 comments)

Acknowledging that from the approved issue
#36771 (comment)
we shall only be accepting: N+Ni format.

Pj, shall we be accepting spaces between the N+Ni?

Thank you.


Please don’t reply on this GitHub thread. Visit golang.org/cl/216617.
After addressing review feedback, remember to publish your drafts!

@pjebs
Copy link
Contributor Author

pjebs commented Apr 12, 2020

no spaces either

- User rune instead of string for comparison
@gopherbot
Copy link
Contributor

This PR (HEAD: 4ec856a) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/216617 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

This PR (HEAD: d0e2ade) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/216617 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

This PR (HEAD: d45bbbb) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/216617 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from pj:

Patch Set 42:

(7 comments)

Patch Set 41:

(1 comment)

Please address the test suite. Thanks.

@robertgriesemer

I just want to say that I've had eye strain for the whole week so I've struggled to be in front of a screen. I knew that you were super busy and this was low-priority so I tried my best to complete the task be the end of the week but it's gotten to the point where I can't seem spend even 15 minutes in front a screen.

It was an absolute honour working with you and learning from the best. I've done most of the tasks. The only remaining task is the hexadecimal test suite. Hopefully you'll be able to take it from here.

Regards,

-PJ


Please don’t reply on this GitHub thread. Visit golang.org/cl/216617.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Robert Griesemer:

Patch Set 45: Run-TryBot+1


Please don’t reply on this GitHub thread. Visit golang.org/cl/216617.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gobot Gobot:

Patch Set 45:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=ade7ab0d


Please don’t reply on this GitHub thread. Visit golang.org/cl/216617.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gobot Gobot:

Patch Set 45:

Build is still in progress...
This change failed on misc-compile-freebsd:
See https://storage.googleapis.com/go-build-log/ade7ab0d/misc-compile-freebsd_b81da872.log

Other builds still in progress; subsequent failure notices suppressed until final report. Consult https://build.golang.org/ to see whether they are new failures. Keep in mind that TryBots currently test exactly your git commit, without rebasing. If your commit's git parent is old, the failure might've already been fixed.


Please don’t reply on this GitHub thread. Visit golang.org/cl/216617.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gobot Gobot:

Patch Set 45: TryBot-Result-1

20 of 20 TryBots failed:
Failed on misc-compile-freebsd: https://storage.googleapis.com/go-build-log/ade7ab0d/misc-compile-freebsd_b81da872.log
Failed on misc-compile-darwin: https://storage.googleapis.com/go-build-log/ade7ab0d/misc-compile-darwin_9c1cd420.log
Failed on misc-compile-mips: https://storage.googleapis.com/go-build-log/ade7ab0d/misc-compile-mips_3355b8f3.log
Failed on misc-compile-linuxarm: https://storage.googleapis.com/go-build-log/ade7ab0d/misc-compile-linuxarm_88a39ef1.log
Failed on js-wasm: https://storage.googleapis.com/go-build-log/ade7ab0d/js-wasm_34fca3bb.log
Failed on misc-compile-other: https://storage.googleapis.com/go-build-log/ade7ab0d/misc-compile-other_79313a85.log
Failed on linux-amd64: https://storage.googleapis.com/go-build-log/ade7ab0d/linux-amd64_76d62f9f.log
Failed on misc-compile-openbsd: https://storage.googleapis.com/go-build-log/ade7ab0d/misc-compile-openbsd_fbcdb079.log
Failed on misc-compile-solaris: https://storage.googleapis.com/go-build-log/ade7ab0d/misc-compile-solaris_6dbb8d49.log
Failed on misc-compile-netbsd: https://storage.googleapis.com/go-build-log/ade7ab0d/misc-compile-netbsd_06fc12dd.log
Failed on misc-compile-ppc: https://storage.googleapis.com/go-build-log/ade7ab0d/misc-compile-ppc_b663d7a5.log
Failed on linux-386: https://storage.googleapis.com/go-build-log/ade7ab0d/linux-386_9b97c894.log
Failed on freebsd-amd64-12_0: https://storage.googleapis.com/go-build-log/ade7ab0d/freebsd-amd64-12_0_c6feebf7.log
Failed on misc-compile-plan9: https://storage.googleapis.com/go-build-log/ade7ab0d/misc-compile-plan9_fdfd57e2.log
Failed on (x/tools) linux-amd64: https://storage.googleapis.com/go-build-log/ade7ab0d/linux-amd64_b8b91384.log
Failed on android-amd64-emu: https://storage.googleapis.com/go-build-log/ade7ab0d/android-amd64-emu_54b1e196.log
Failed on windows-amd64-2016: https://storage.googleapis.com/go-build-log/ade7ab0d/windows-amd64-2016_101db865.log
Failed on windows-386-2008: https://storage.googleapis.com/go-build-log/ade7ab0d/windows-386-2008_1d2c454c.log
Failed on openbsd-amd64-64: https://storage.googleapis.com/go-build-log/ade7ab0d/openbsd-amd64-64_6cc4e370.log
Failed on linux-amd64-race: https://storage.googleapis.com/go-build-log/ade7ab0d/linux-amd64-race_66b2b325.log

Consult https://build.golang.org/ to see whether they are new failures. Keep in mind that TryBots currently test exactly your git commit, without rebasing. If your commit's git parent is old, the failure might've already been fixed.


Please don’t reply on this GitHub thread. Visit golang.org/cl/216617.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Robert Griesemer:

Patch Set 45:

LGTM.

Thanks for sticking with this, and I'm sorry to hear about your eye strain. Safety and health take priority of course, so don't worry about this CL.

I've reviewed this once more and - as you say, some tests could be trimmed some more - it looks good. Also, the test file needs to be gofmt-ed.

I'm going to make some of these minor corrections and will take it from here. Thanks again.


Please don’t reply on this GitHub thread. Visit golang.org/cl/216617.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Robert Griesemer:

Patch Set 45:

(1 comment)

I've cleaned up the test suite and made it more regular. I am going to attach it


Please don’t reply on this GitHub thread. Visit golang.org/cl/216617.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

This PR (HEAD: d336057) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/216617 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Robert Griesemer:

Patch Set 46: Run-TryBot+1


Please don’t reply on this GitHub thread. Visit golang.org/cl/216617.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gobot Gobot:

Patch Set 46:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=bfc34b44


Please don’t reply on this GitHub thread. Visit golang.org/cl/216617.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gobot Gobot:

Patch Set 46:

Build is still in progress...
This change failed on js-wasm:
See https://storage.googleapis.com/go-build-log/bfc34b44/js-wasm_e6bf964e.log

Other builds still in progress; subsequent failure notices suppressed until final report. Consult https://build.golang.org/ to see whether they are new failures. Keep in mind that TryBots currently test exactly your git commit, without rebasing. If your commit's git parent is old, the failure might've already been fixed.


Please don’t reply on this GitHub thread. Visit golang.org/cl/216617.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gobot Gobot:

Patch Set 46: TryBot-Result-1

20 of 20 TryBots failed:
Failed on js-wasm: https://storage.googleapis.com/go-build-log/bfc34b44/js-wasm_e6bf964e.log
Failed on linux-amd64: https://storage.googleapis.com/go-build-log/bfc34b44/linux-amd64_40b077e7.log
Failed on (x/tools) linux-amd64: https://storage.googleapis.com/go-build-log/bfc34b44/linux-amd64_94878302.log
Failed on freebsd-amd64-12_0: https://storage.googleapis.com/go-build-log/bfc34b44/freebsd-amd64-12_0_c38967ea.log
Failed on openbsd-amd64-64: https://storage.googleapis.com/go-build-log/bfc34b44/openbsd-amd64-64_364ca300.log
Failed on windows-amd64-2016: https://storage.googleapis.com/go-build-log/bfc34b44/windows-amd64-2016_e108f387.log
Failed on android-amd64-emu: https://storage.googleapis.com/go-build-log/bfc34b44/android-amd64-emu_ad73a894.log
Failed on misc-compile-solaris: https://storage.googleapis.com/go-build-log/bfc34b44/misc-compile-solaris_0f498d7b.log
Failed on misc-compile-mips: https://storage.googleapis.com/go-build-log/bfc34b44/misc-compile-mips_c440594d.log
Failed on misc-compile-plan9: https://storage.googleapis.com/go-build-log/bfc34b44/misc-compile-plan9_2545f6b2.log
Failed on misc-compile-openbsd: https://storage.googleapis.com/go-build-log/bfc34b44/misc-compile-openbsd_4515091e.log
Failed on misc-compile-ppc: https://storage.googleapis.com/go-build-log/bfc34b44/misc-compile-ppc_49eba3d3.log
Failed on misc-compile-other: https://storage.googleapis.com/go-build-log/bfc34b44/misc-compile-other_0ac04370.log
Failed on misc-compile-freebsd: https://storage.googleapis.com/go-build-log/bfc34b44/misc-compile-freebsd_410a41d4.log
Failed on misc-compile-linuxarm: https://storage.googleapis.com/go-build-log/bfc34b44/misc-compile-linuxarm_39b4fb46.log
Failed on linux-386: https://storage.googleapis.com/go-build-log/bfc34b44/linux-386_45861702.log
Failed on misc-compile-netbsd: https://storage.googleapis.com/go-build-log/bfc34b44/misc-compile-netbsd_f894d9b0.log
Failed on linux-amd64-race: https://storage.googleapis.com/go-build-log/bfc34b44/linux-amd64-race_f3e529c0.log
Failed on misc-compile-darwin: https://storage.googleapis.com/go-build-log/bfc34b44/misc-compile-darwin_1a463404.log
Failed on windows-386-2008: https://storage.googleapis.com/go-build-log/bfc34b44/windows-386-2008_34ff4c1f.log

Consult https://build.golang.org/ to see whether they are new failures. Keep in mind that TryBots currently test exactly your git commit, without rebasing. If your commit's git parent is old, the failure might've already been fixed.


Please don’t reply on this GitHub thread. Visit golang.org/cl/216617.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from pj:

Patch Set 46:

Patch Set 46: TryBot-Result-1

20 of 20 TryBots failed:
Failed on js-wasm: https://storage.googleapis.com/go-build-log/bfc34b44/js-wasm_e6bf964e.log
Failed on linux-amd64: https://storage.googleapis.com/go-build-log/bfc34b44/linux-amd64_40b077e7.log
Failed on (x/tools) linux-amd64: https://storage.googleapis.com/go-build-log/bfc34b44/linux-amd64_94878302.log
Failed on freebsd-amd64-12_0: https://storage.googleapis.com/go-build-log/bfc34b44/freebsd-amd64-12_0_c38967ea.log
Failed on openbsd-amd64-64: https://storage.googleapis.com/go-build-log/bfc34b44/openbsd-amd64-64_364ca300.log
Failed on windows-amd64-2016: https://storage.googleapis.com/go-build-log/bfc34b44/windows-amd64-2016_e108f387.log
Failed on android-amd64-emu: https://storage.googleapis.com/go-build-log/bfc34b44/android-amd64-emu_ad73a894.log
Failed on misc-compile-solaris: https://storage.googleapis.com/go-build-log/bfc34b44/misc-compile-solaris_0f498d7b.log
Failed on misc-compile-mips: https://storage.googleapis.com/go-build-log/bfc34b44/misc-compile-mips_c440594d.log
Failed on misc-compile-plan9: https://storage.googleapis.com/go-build-log/bfc34b44/misc-compile-plan9_2545f6b2.log
Failed on misc-compile-openbsd: https://storage.googleapis.com/go-build-log/bfc34b44/misc-compile-openbsd_4515091e.log
Failed on misc-compile-ppc: https://storage.googleapis.com/go-build-log/bfc34b44/misc-compile-ppc_49eba3d3.log
Failed on misc-compile-other: https://storage.googleapis.com/go-build-log/bfc34b44/misc-compile-other_0ac04370.log
Failed on misc-compile-freebsd: https://storage.googleapis.com/go-build-log/bfc34b44/misc-compile-freebsd_410a41d4.log
Failed on misc-compile-linuxarm: https://storage.googleapis.com/go-build-log/bfc34b44/misc-compile-linuxarm_39b4fb46.log
Failed on linux-386: https://storage.googleapis.com/go-build-log/bfc34b44/linux-386_45861702.log
Failed on misc-compile-netbsd: https://storage.googleapis.com/go-build-log/bfc34b44/misc-compile-netbsd_f894d9b0.log
Failed on linux-amd64-race: https://storage.googleapis.com/go-build-log/bfc34b44/linux-amd64-race_f3e529c0.log
Failed on misc-compile-darwin: https://storage.googleapis.com/go-build-log/bfc34b44/misc-compile-darwin_1a463404.log
Failed on windows-386-2008: https://storage.googleapis.com/go-build-log/bfc34b44/windows-386-2008_34ff4c1f.log

Consult https://build.golang.org/ to see whether they are new failures. Keep in mind that TryBots currently test exactly your git commit, without rebasing. If your commit's git parent is old, the failure might've already been fixed.

You can't run the try bot until you merge parseFloatPrefix.


Please don’t reply on this GitHub thread. Visit golang.org/cl/216617.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

This PR (HEAD: ef5b113) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/216617 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

This PR (HEAD: 036a075) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/216617 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Dmitri Shuralyov:

Patch Set 48: Run-TryBot+1


Please don’t reply on this GitHub thread. Visit golang.org/cl/216617.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gobot Gobot:

Patch Set 48:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=9ee43817


Please don’t reply on this GitHub thread. Visit golang.org/cl/216617.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gobot Gobot:

Patch Set 48: TryBot-Result+1

TryBots are happy.


Please don’t reply on this GitHub thread. Visit golang.org/cl/216617.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Dmitri Shuralyov:

Patch Set 48:

(2 comments)

Minor comments on the commit message. https://golang.org/wiki/CommitMessage has more information on the style used by the Go project. Thanks!


Please don’t reply on this GitHub thread. Visit golang.org/cl/216617.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Dmitri Shuralyov:

Patch Set 49:

pj, a better link about editing the commit message is https://golang.org/wiki/CommitMessage#github-pull-requests (bottom of the page). You'll need to use the Edit button in the top right corner of #36815 to edit the Pull Request subject.


Please don’t reply on this GitHub thread. Visit golang.org/cl/216617.
After addressing review feedback, remember to publish your drafts!

@pjebs pjebs changed the title strconv: Add ParseComplex and FormatComplex strconv: add ParseComplex and FormatComplex May 8, 2020
gopherbot pushed a commit that referenced this pull request May 8, 2020
Adds two functions to deal with complex numbers:
* FormatComplex
* ParseComplex

ParseComplex accepts complex numbers in this format: N+Ni

Fixes #36771

Change-Id: Id184dc9e277e5fa01a714ad656a88255ead05085
GitHub-Last-Rev: 036a075
GitHub-Pull-Request: #36815
Reviewed-on: https://go-review.googlesource.com/c/go/+/216617
Reviewed-by: Robert Griesemer <gri@golang.org>
@gopherbot
Copy link
Contributor

Message from Robert Griesemer:

Patch Set 50: Code-Review+2


Please don’t reply on this GitHub thread. Visit golang.org/cl/216617.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

This PR is being closed because golang.org/cl/216617 has been merged.

@gopherbot gopherbot closed this May 8, 2020
@pjebs pjebs deleted the strconv.ParseCompex branch May 9, 2020 10:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Used by googlebot to label PRs as having a valid CLA. The text of this label should not change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

strconv: add ParseComplex
3 participants