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.

Sign up for GitHub

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

Jump to bottom

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

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

dr2chase opened this issue Sep 10, 2018 · 9 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.

Comments

@dr2chase
Copy link
Contributor

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
@josharian josharian added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Sep 10, 2018
@mark-rushakoff
Copy link
Contributor

I think that commit was 7c7cecc.

@dr2chase
Copy link
Contributor Author

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

@mark-rushakoff
Copy link
Contributor

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 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

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 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

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

But, WAI, not a bug.

@kortschak
Copy link
Contributor

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.

@mvdan mvdan mentioned this issue Sep 12, 2018
Closed
@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.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
7 participants
@josharian @kortschak @mark-rushakoff @dr2chase @mvdan @robpike @gopherbot