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

1.0 listed as not-integer #22

Closed
awwright opened this issue Feb 1, 2013 · 9 comments
Closed

1.0 listed as not-integer #22

awwright opened this issue Feb 1, 2013 · 9 comments

Comments

@awwright
Copy link
Member

awwright commented Feb 1, 2013

e39d537 added a test saying that, apparently, 1.0 should not validate as an integer.

This is a problem, most notably, for Javascript/ECMAScript (I wonder where JSON comes from), where everything is an IEEE 64-bit floating point, and fails against my library since this is listed as a required test even though ES itself has no other alternative.

Differentiation between floating point and non-floating-point forms is listed as an optional (actually, suggested) part of v4, and is not mentioned at all in v3.

This test should, accordingly, be moved to the "optional tests" section.

@fge
Copy link
Contributor

fge commented Feb 1, 2013

1.0 is indeed not an integer as per section 3.5 of the core spec which defines an integer as "a number without any fractional or exponent part". RFC 4627, section 2.4:

number = [ minus ] int [ frac ] [ exp ]

The integer removes the frac and exp parts from the picture. It is far from being optional!

This test is therefore normal and should not be moved to "optional" since the vast majority of languages do differentiate between integers and floating point numbers (even Python).

Let us say that it is a test that is expected to fail for JS based validators... Unless someone has actually found out how to do it!

@awwright
Copy link
Member Author

awwright commented Feb 1, 2013

JSON Schema semantics defines otherwise:

It is acknowledged by this specification that some programming languages, and their associated parsers, use different internal representations for floating point numbers and integers, while others do not.

As a consequence, for interoperability reasons, JSON values used in the context of JSON Schema, whether that JSON be a JSON Schema or an instance, SHOULD ensure that mathematical integers be represented as integers as defined by this specification.

The RFC has to distinguish between the two forms because they're different encodings of the same value. But while we're at it, let's also add a tests and a JSON Schema type for "exponent", too:

exp = e [ minus / plus ] 1*DIGIT

I mean NO, of course not, that's silly. If we're going to be citing the JSON RFC as the final authority then let's also add tests to make sure we can't validate application/json documents that aren't arrays or objects, either.

@fge
Copy link
Contributor

fge commented Feb 1, 2013

Well, an exponent is a part of a number. I mean, 1.298e+232 is legal, right?

OK, it's going to be moved to optional. But really, this JavaScript brain damaged number handling is a royal PITA.

And anyway, I suppose JavaScript is never going to be used in finance, so at least they are safe :p

fge added a commit that referenced this issue Feb 1, 2013
Thanks to some brain damaged languages, and given what section 5.5 says, this
test has to go in this section.

What on earth was ECMA thinking, I wonder.

Solves issue #22.
@fge fge closed this as completed Feb 1, 2013
@awwright
Copy link
Member Author

awwright commented Feb 1, 2013

I might suggest a slightly more descriptive filename, maybe "floatNotInt", otherwise people who want to include this optional test (which should be plenty of them) would see "jsBrainDamage" which makes it sound like it doesn't apply to them.

I was wondering why I was getting a syntax error even though GitHub says it passed, I was going to fix the error but it looks like you got it after all.

Thanks!

fge added a commit that referenced this issue Feb 2, 2013
I am overreacting on such matters.

See issue #22.
@fge
Copy link
Contributor

fge commented Feb 2, 2013

OK, renamed to zeroTerminatedFloats.json

@awwright
Copy link
Member Author

awwright commented Feb 2, 2013

Perfect

@geraintluff
Copy link
Contributor

It still refers to it as "brain damaged" inside the file.

Change it, Francis.

@geraintluff
Copy link
Contributor

Never mind, I've done it.

Look - I know that you're primarily a Java programmer, and Java deals with numeric types a certain way. And that is appropriate (even necessary) for some situations.

But the distinction between floating-point and integer is one of internal representation. If a language (JS is not the only one) does not treat that internal representation as an essential aspect of the numeric value, then that is also a valid decision.

There is no need to continue your crusade all the way to naming tests.

@Julian
Copy link
Member

Julian commented Feb 3, 2013

Yes, sorry, forgot about languages without the distinction. This is good (though I'd prefer it to have been done on develop, I'll merge it there now). We need a page discussing how optional the optional tests are now though. I'll open that as a separate ticket.

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

4 participants