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

Cannot decode integer value to float64 type. #60

Closed
bmatsuo opened this issue Sep 9, 2014 · 14 comments
Closed

Cannot decode integer value to float64 type. #60

bmatsuo opened this issue Sep 9, 2014 · 14 comments
Assignees

Comments

@bmatsuo
Copy link

bmatsuo commented Sep 9, 2014

It seems like a struct containing a float64 field cannot be decoded from a TOML document containing an integer value. The following example demonstrates this.

package main

import (
    "log"

    "github.com/BurntSushi/toml"
)

func main() {
    raw := "Foo = 123"
    var s struct {
        Foo float64
    }
    _, err := toml.Decode(raw, &s)
    if err != nil {
        log.Fatal(err)
    }
    log.Print(s)
}

The error message printed looks like this

2014/09/09 13:42:51 Type mismatch for 'struct { Foo float64 }.Foo': Expected float but found 'int64'.

@BurntSushi
Copy link
Owner

Hmm. This appears to be working as intended. In TOML, floats are indicated by the presence of a decimal point and at least one trailing 0.

I think I would rather improve the error message (which looks pretty cryptic from the perspective of the user) than change the behavior.

@bmatsuo
Copy link
Author

bmatsuo commented Sep 9, 2014

My reading of the spec did not leaving feeling like this behavior is actually dictated. Despite differentiating their syntax it does not say they cannot be interchanged. There are of course languages which do not have native integer types (javascript), but that's another issue..

Anyway, IMO it's incorrect. If not technically incorrect than practically incorrect in that it is unexpected and it makes the format more troublesome to work with.

The spec does say to "do what's natural" with integers. It seems to be some joke about natural numbers. (edit: the cheeky comment has been removed from the spec) But seriously, this lossless conversion between numeric types is the natural action in my mind. Natural enough that I reported this as a bug ;)

Regardless, the error message is a bit strange.

@cespare cespare self-assigned this Feb 23, 2016
@cespare
Copy link
Collaborator

cespare commented Feb 23, 2016

I think the behavior is fine as-is. TOML and Go both distinguish between integer and floating-point types; I think it makes sense to enforce that distinction when decoding.

But seriously, this lossless conversion between numeric types is the natural action in my mind.

It is not a lossless conversion. For example, 10000000000000001 is a valid TOML integer, and a valid Go int64, but cannot be precisely represented as a 64-bit float.

I'll see about improving the error message.

@bmatsuo
Copy link
Author

bmatsuo commented Feb 26, 2016

@cespare, indeed the spec allows for numbers to exceed the capacity of destination type. It claims (as of this date) to "expect" values within a range/precision. But that is quite different from saying they are invalid. Clearly problems with integer ranges exist with deserialization into other types like int32 and uint. All I would propose is that a conversion to float64 or float32 be handled the same way.

Apparently no other users think as I do given how long this issue has gone on without another supporting comment. So I guess you can do what you will with it. But I still think the package's behavior is incorrect.

If a user is trying to use the Foobaz command with a TOML config file they shouldn't have to wonder if Foobaz is implemented in a language with static types or not when setting numerical configuration values. I think it is just bad UX.

But like I said, if no one agrees then you can close this issue if you want.

@cespare
Copy link
Collaborator

cespare commented Feb 26, 2016

I don't think either way is "correct" or "incorrect". The TOML spec doesn't give any precise rules about how TOML types have to map to language types. In Go it's natural for TOML integers to map to Go integer types and for TOML floats to map to Go floating-point types.

You're asking for a relaxation which makes certain (uncommon?) code more convenient.

I'm not really opposed to the change, but I don't see this as a failing or bug of the library. If you'd like to send along a PR, I'd be happy to review it. Otherwise, I'll see about improving the error message myself.

cespare added a commit that referenced this issue Jul 7, 2016
Some highlights:

- Don't start errors with capital letters or end with periods
- Add 'toml: ' prefix
- Get rid of a recursive Decode error: this becomes excessively long
  without being all that helpful for troubleshooting
- Reword some messages

Updates #60.
@cespare
Copy link
Collaborator

cespare commented Jul 7, 2016

The error message is now

toml: cannot decode TOML float into Go value of type int64

I think this is sufficiently clear.

@cespare cespare closed this as completed Jul 7, 2016
@cespare
Copy link
Collaborator

cespare commented Jul 7, 2016

Sorry, that error message was reversed of what it should have been. I've fixed it and now the error reads

toml: cannot load TOML value of type int64 into a Go float

mitszo pushed a commit to accense/toml that referenced this issue Jul 19, 2016
Some highlights:

- Don't start errors with capital letters or end with periods
- Add 'toml: ' prefix
- Get rid of a recursive Decode error: this becomes excessively long
  without being all that helpful for troubleshooting
- Reword some messages

Updates BurntSushi#60.
@AndreKR
Copy link

AndreKR commented Nov 3, 2017

From a user perspective I cannot understand why I cannot set a float value as foo = 3 but have to write foo = 3.0. That makes absolutely no sense and there is no other application that forces me to write .0.

@hgl
Copy link

hgl commented Dec 6, 2017

Just encountered this one too. Agree with @AndreKR, as a user, this limitation is quite frustrating.

The official json package allows decoding to either int or float for a json number. I think toml should behave the same.

@kevinlebrun
Copy link

kevinlebrun commented Apr 11, 2018

Same issue here, it is totally unintuitive and actually error-prone when editing configuration files.

@BurntSushi
Copy link
Owner

+1 comments on issues are rude. If you don't have any new perspective to offer, then don't comment.

@kevinlebrun
Copy link

kevinlebrun commented Apr 11, 2018

I didn't mean to be rude here. I just lost time and took a risk to introduce a serious issue in our configuration system because of this rule. I could have created another ticket but as it is an opinion I found contributing to this one a better option.

If someone encounter the same problem I managed to force a cast to float using a custom type with UmarshalTOML.

@BurntSushi
Copy link
Owner

You're not adding anything new. You're just saying "me too." It isn't helpful and just adds noise.

@saracen
Copy link
Contributor

saracen commented Nov 10, 2021

I've opened #325 which I believe addresses the concerns here that integers cannot be represented losslessly as floats. Some integers can be represented losslessly and I think it makes sense to allow those and is in keeping with the specification.

@BurntSushi I'd appreciate if you could take a look and let me know if you're convinced by my argument. If not, then I apologise for the noise. We stumbled across this problem in GitLab-Runner recently and this seemed easier than going the route of supporting a special IntOrFloat type.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants