strconv: add ParseComplex

[pjebs]

strconv.ParseComplex(s string, bitSize int) (complex128, error)

[pjebs]

I don't think it should import regexp or strings to keep the package small. It is heavily recommended (instead of fmt) by gopherjs for exactly this reason.

[josharian]

Exactly what format(s) do you think it should accept?

[ianlancetaylor]

Note that the language doesn't define any format for a complex expression. Instead it defines an imaginary literal (an integer or floating point constant followed by i). Any complex constant is simply a constant expression that includes at least one imaginary literal. So I think @josharian is asking a good question here. We don't want the strconv package to have to parse an arbitrary expression.

That said, we do have an implicit definition for parsing a complex constant in fmt.Scan, implemented in the method fmt.(*ss).complexTokens (https://golang.org/src/fmt/scan.go#L740). This syntax doesn't seem to have any user-visible documentation. We could decide to move that method over to strconv.ParseComplex.

[pjebs]

Let me know with what I've done so far after addressing your concerns.

PR: #36815

[pjebs]

It will accept:

- (±34.3±234.2i)
- ±34.3±234.2i
- exponential form
- hexadecimal form
- NaN
- ±Inf±Infi
- only real
- only imag

[gopherbot]

Change https://golang.org/cl/216617 mentions this issue: strconv: Add ParseCompex function

[pjebs]

Any news on this?

[rsc]

fmt.Print and fmt.Scan do have an implicit format for complex numbers:
FLOAT+/-FLOATi or (FLOAT+/-FLOATi)
They format and accept both decimal and hex floats, and NaN and Inf for either or both FLOATs.

For floats we have:

func ParseFloat(s string, bitSize int) (float64, error)
func FormatFloat(f float64, fmt byte, prec, bitSize int) string

It seems like we'd need

func ParseComplex(s string, bitSize int) (complex128, error)
func FormatComplex(c complex128, fmt byte, prec, bitSize int) string

This seems like it is a hole we forgot to fill.

/cc @robpike @griesemer

[robpike]

Seems like a reasonable thing to add, in parallel with float64 support as @rsc says. It would be good to see if the fmt package adapts well to using it.

[pjebs]

What should FormatComplex return?

• With brackets or without brackets?
• hex or always decimal?

[rsc]

FormatComplex should probably put in the parens, same as fmt.
The question of hex or decimal is controlled by the fmt byte,
like in Printf: 'x' gets hex.

For example, I believe these tests from fmt show what fmt='x' prec=3 should do:

fmt_test.go:532:	{"%.3x", 0i, "(0x0.000p+00+0x0.000p+00i)"},
fmt_test.go:538:	{"%.3x", 1 + 2i, "(0x1.000p+00+0x1.000p+01i)"},
fmt_test.go:542:	{"%.3x", -1 - 2i, "(-0x1.000p+00-0x1.000p+01i)"},

[pjebs]

cool. implemented FormatComplex. Will do unit tests later.

[rsc]

Based on the discussion, this seems like a likely accept.

[rsc]

No change in consensus, so accepted.

[pjebs]

So what happens now? Well it be in go 1.15?

[pjebs]

Is it possible to keep strings package out because it's heavily used in environments like gopher.js @odeke-em

[odeke-em]

For sure, @pjebs, thanks for letting me know :) Let's take it to your CL, where I've posted feedback.

[pjebs]

@odeke-em Thank you for the feedback. I'll make various adjustments in accordance with your suggestions. I was told we were only accepting (N±Ni) format as per #36771 (comment)

[odeke-em]

Gotcha! My interpretation of that comment was more of: the format isn’t defined and yes we shouldn’t be accepting arbitrary expressions. However the Go parser and compilers accept either of these forms for complex constants: REAL JOINING_SIGN IMAG IMAG JOINING_SIGN REAL So I think that naturally Go users will have an expectation of being able to use any of: -30-50i -50i-30 Or: -50i Or: -30 As well as other forms of IEEE 754 floats for the constants. As Ian mentioned a complex is just a constant affixed with an “i”. Your CL currently accepts: -30-50i but not -50i-30. I think we can make it accept the latter form, by meeting in the middle for a hybrid of suggestions.

…

On Sat, Apr 11, 2020 at 7:21 PM pj ***@***.***> wrote: @odeke-em <https://github.com/odeke-em> Thank you for the feedback. I'll make various adjustments. I was told we were only accepting (N=Ni) format as per #36771 (comment) <#36771 (comment)> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#36771 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABFL3V2XBQYA6D42ME45UB3RMEQUBANCNFSM4KLT6XNQ> .

[pjebs]

I got confused because the @ianlancetaylor pointed me to the "implicit definition" that Go currently accepts and it states:

// complexTokens returns the real and imaginary parts of the complex number starting here.
// The number might be parenthesized and has the format (N+Ni) where N is a floating-point
// number and there are no spaces within.

My interpretation was that there is already code that only parses/accepts (N+Ni) and @ianlancetaylor wants to replace that existing code (in fmt) with the new strconv.ParseComplex, in which case it will create a break to the Go1 backward compatibility policy.

[ianlancetaylor]

The format we accepted for this proposal is exactly the one in #36771 (comment) , which is simpler and more constraining than what @odeke-em suggests above.

[odeke-em]

Thank you for the clarification on the interpretation and what was accepted, Ian. Alright, in the future after we’ve merged the related CL. If users request for it, would it be logical for us to accept Ni+N as well?

…

On Sat, Apr 11, 2020 at 8:10 PM Ian Lance Taylor ***@***.***> wrote: The format we accepted for this proposal is exactly the one in #36771 (comment) <#36771 (comment)> , which is simpler and more constraining than what @odeke-em <https://github.com/odeke-em> suggests above. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#36771 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABFL3V2UXNKMFQUTYKXBH53RMEWMFANCNFSM4KLT6XNQ> .

[pjebs]

In that case, @odeke-em can you remove the comments you made that are related to the expanded definition. I believe there were only cosmetic adjustments left.

[odeke-em]

Acknowledged, thank you @pjebs and @ianlancetaylor for the clarification. I shall update my comments on the CL.

[pjebs]

@odeke-em Finished amending PR based on your comments.

The only thing I didn't change was your suggestion that I change IEEE754 to IEEE 754 because the ParseComplex doc comments are directly from ParseFloat function and that's how it was in there.

Thanks for the review.
Also noticed to my surprise that you star'd my https://github.com/rocketlaunchr/dbq package.

[odeke-em]

@odeke-em Finished amending PR based on your comments.

Awesome, I shall take a look and I'll also kindly rope in @griesemer for the final look.

Also noticed to my surprise that you star'd my https://github.com/rocketlaunchr/dbq package.

Aye aye. Nice work and ergonimcs, I hope to use it sometime in the future!

[pjebs]

The code already (implicitly) rejects malformed parenthesis

[pjebs]

@odeke-em

Issues:

PS8, Line 10
This suggestion wasn't added.

Am I meant to change the commit message for my initial commit? Is that how I do it?

We need to ensure that malformed parenthesis throw out an error e.g.:

The ParseFloat function already takes care of rejecting malformed parenthesis. There is no need to cater for it explicitly. See test cases.

This code isn't intuitive to me on what it does.
Would be nice to explain the logic of how we figure out what's real what's imaginary.

I have added more detailed comments to explain the logic. I feel that the algorithmic approach is simpler once the logic is understood.

[odeke-em]

Thanks for the ping Pj! Please use the CL or the respective PR to respond to code review comments otherwise we now no longer have threaded conversations related to the code. Please add those comments to the PR or better, even log in and comment directly on the CL. I can’t correlate them via email and the actual CL :)

…

On Mon, Apr 20, 2020 at 8:34 PM pj ***@***.***> wrote: @odeke-em <https://github.com/odeke-em> Issues: PS8, Line 10 This suggestion wasn't added. Am I meant to change the commit message for my initial commit? Is that how I do it? We need to ensure that malformed parenthesis throw out an error e.g.: The ParseFloat function already takes care of rejecting malformed parenthesis. There is no need to cater for it. See test cases. This code isn't intuitive to me on what it does. Would be nice to explain the logic of how we figure out what's real what's imaginary. I have added more detailed comments to explain the logic. I feel that the algorithmic approach is simpler once the logic is understood. Hexadecimal values e.g. ±0x10.3p±8±0x3p3i *The ParseComplex function leverages on ParseFloat. I can't get even ParseFloat to accept hexadecimal values. See: https://play.golang.org/p/E1s39kCdmbV <https://play.golang.org/p/E1s39kCdmbV>* Can you please look at the playground and tell me what I'm doing wrong? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#36771 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABFL3V6IVZGCDXKV72XU35LRNUH57ANCNFSM4KLT6XNQ> .

[pjebs]

@odeke-em I responded on the official site.

[pjebs]

@ianlancetaylor @griesemer Will this make it to Go1.15?

[griesemer]

I've started looking at the code.

[gopherbot]

Change https://golang.org/cl/230737 mentions this issue: strconv: implement parseFloat returning no. of bytes consumed

[gopherbot]

Change https://golang.org/cl/231237 mentions this issue: strconv: fix for parseFloatPrefix