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

fmt: %#v of integer value in float64 (fraction = 0) incorrectly has ".0" suffix #27598

Closed
dr2chase opened this issue Sep 10, 2018 · 9 comments
Closed

Comments

@dr2chase
Copy link
Contributor

@dr2chase dr2chase commented Sep 10, 2018

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

Tip.
go version devel +7a0eb56466 Fri Sep 7 17:28:27 2018 +0000 darwin/amd64

Does this issue reproduce with the latest release?

Not with 1.11, but with current Tip.

What operating system and processor architecture are you using (go env)?

amd64, OSX

What did you do?

It's okay in the playground: https://play.golang.org/p/-tWXNCOTgVX

package main

import "fmt"

var x float64 = 1

func main() {
	fmt.Printf("Hello x, %v, %#v\n", x, x)
}

What did you expect to see?

Hello x, 1, 1

What did you see instead?

Hello x, 1, 1.0

I found this running the gonum tests. There it fails like so: (CC @btracey)

    format_test.go:146: unexpected format result test 0 part 2:
        got:
        &mat.Dense{mat:blas64.General{Rows:3, Cols:3, Data:[]float64{0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0}, Stride:3}, capRows:3, capCols:3}
        want:
        &mat.Dense{mat:blas64.General{Rows:3, Cols:3, Data:[]float64{0, 0, 0, 0, 0, 0, 0, 0, 0}, Stride:3}, capRows:3, capCols:3}
    format_test.go:146: unexpected format result test 1 part 2:
        got:
        &mat.Dense{mat:blas64.General{Rows:3, Cols:3, Data:[]float64{1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0}, Stride:3}, capRows:3, capCols:3}
        want:
        &mat.Dense{mat:blas64.General{Rows:3, Cols:3, Data:[]float64{1, 1, 1, 1, 1, 1, 1, 1, 1}, Stride:3}, capRows:3, capCols:3}
(etc)
@josharian josharian changed the title Format %#v of integer value in float64 (fraction = 0) incorrectly has ".0" suffix fmt: %#v of integer value in float64 (fraction = 0) incorrectly has ".0" suffix Sep 10, 2018
@mark-rushakoff
Copy link
Contributor

@mark-rushakoff mark-rushakoff commented Sep 10, 2018

I think that commit was 7c7cecc.

@dr2chase
Copy link
Contributor Author

@dr2chase dr2chase commented Sep 10, 2018

Oh, that would be it. So this is NOT a bug?

@mark-rushakoff
Copy link
Contributor

@mark-rushakoff mark-rushakoff commented Sep 10, 2018

I should have linked to the original issue, #26363. @robpike said

I think it's a bug and should be fixed.

@robpike
Copy link
Contributor

@robpike robpike commented Sep 10, 2018

Working as intended, now. If you use %#v you should get something that is recognizably a float.

@kortschak
Copy link
Contributor

@kortschak kortschak commented Sep 11, 2018

This is a significant irritation. I am wondering how to deal with testing the case that @dr2chase points to without having to duplicate the matrix formatting test code over the period where we are supporting Go versions that are before and after this change.

@mvdan
Copy link
Member

@mvdan mvdan commented Sep 11, 2018

@kortschak how many releases of Go do your code and tests support? It may be possible to skip the few tests that lock in this behavior on older versions of Go.

Alternatively, is there no way to rewrite the tests to not lock in this particular behavior? For example, you could use a list of acceptable outputs, adding two entries for these cases. Or perhaps even use regular expressions, although that might make the test cases trickier to work with.

@kortschak
Copy link
Contributor

@kortschak kortschak commented Sep 11, 2018

We support current release plus the previous two.

Skipping these tests without losing the whole formatting test suite would involve rewriting the tests significantly, and not locking in the behaviour loses us tests on behaviour that is tenuous anyway because of the unfriendliness of fmt.State use in fmt.Formatter.

Using regexs for this is a horrible thing to have to do.

It seems to me that the easiest way to deal with this is to just duplicate the test file and guard it with tag when that become possible (go1.12beta1 presumably). Until then we just see master tests as noise.

@dr2chase
Copy link
Contributor Author

@dr2chase dr2chase commented Sep 11, 2018

But, WAI, not a bug.

@dr2chase dr2chase closed this Sep 11, 2018
@kortschak
Copy link
Contributor

@kortschak kortschak commented Sep 11, 2018

The behaviour that existed prior to 7c7cecc has been around since Go1 and I would have thought subject to the Go1 promise. Similar arguments have retained things like behaviour of sort order and PRNG sequence.

Further, representing an integral float value without the decimal point is still valid Go syntax. Sure, you cannot correctly type infer from the output, but Go syntax formatting by fmt cannot be guaranteed to output a valid Go value anyway.

@golang golang locked and limited conversation to collaborators Sep 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
7 participants
You can’t perform that action at this time.