test validation of keywords which expect integers when their value has a decimal

[notEthan]

to the best of my understanding this should be valid ("the data model makes no distinction between integers and other numbers" -core 4.2.1).

my implementation was incorrectly considering this to be a schema error.

[Julian]

Seems reasonable.

[ssilverman]

Why just “minItems”? Should we add these tests for all integer-requiring keywords?

[notEthan]

@ssilverman see the description

I'm throwing this up as a draft and can expand it (to other drafts and other keywords that expect integer values) if others reckon it seems a worthwhile addition.

(about to change as I push this out of draft though)

[notEthan]

it appears this change has caught the sanity checker out on the question of whether 1.0 is an integer.

Failed validating 'type' in metaschema['properties']['minItems']:
    {'default': 0, 'minimum': 0, 'type': 'integer'}

On schema['minItems']:
    1.0

[Julian]

That's correctly failing, the test isn't valid on draft4 (where indeed 1.0 is not an integer and it must be).

[notEthan]

removed tests from drafts 3 and 4

[ssilverman]

LGTM

[Relequestual]

core 4.2.1 is describing the data model for the INSTANCE, not the schema.

For consistency, integer JSON numbers SHOULD NOT be encoded with a fractional part.

https://json-schema.org/draft/2019-09/json-schema-core.html#rfc.section.6.3

Core doesn't say anything else about zero fractional parts.
Validation spec doc however, on type...

String values MUST be one of the six primitive types ("null", "boolean", "object", "array", "number", or "string"), or "integer" which matches any number with a zero fractional part.

https://json-schema.org/draft/2019-09/json-schema-validation.html#rfc.section.6.1.1

Further...

The value of "multipleOf" MUST be a number, strictly greater than 0.

https://json-schema.org/draft/2019-09/json-schema-validation.html#rfc.section.6.2.1

The value of this keyword MUST be a non-negative integer. (relating to minItems)

https://json-schema.org/draft/2019-09/json-schema-validation.html#rfc.section.6.4.2

The validation spec DOES define different requirements for different keywords explicityly, and that is a "MUST".

[notEthan]

I'm not sure if you're saying this PR is incorrect, or adding clarification.

my interpretation of the spec, including what you've pointed to, is that 1.0 is an integer.

is there a separate data model for schemas? schemas being instances themselves, of the metaschema. the metaschema says these keywords are type: integer and the spec says the integer type matches a 0 fractional part.

[notEthan]

hit the wrong button ...

[Relequestual]

I'm not convinced this PR is valid, but I'm willing to be convinced (hence this wasn't a review).

As soon as a schema becomes an instance, it's then an instance and not a schema, and so follows the data model of an instance (which makes no distinction with 0 fractional parts).

My take on this is:

• The spec says when validating an instance, 0 fractional parts are equivilent to integers.
• The spec says that maxContains for example must be an integer.
• The spec DOES NOT say that the SCHEMA data model must accept 0 fractional parts to be equivilent as integers, and therefore, my feeling is it's wrong to write a test to say it MUST be something that's supported.

This could result in a schema being valid according to the meta-schema, but not according to an implementation, because the schema has a 0 fractional part where an interger was expected.
I think THIS IS FINE, because we even hint at this being a posbility in 2019-09 core#6.3.

[notEthan]

the fact that the language is a SHOULD NOT instead of a MUST NOT in core 6.3 "integer JSON numbers SHOULD NOT be encoded with a fractional part" seems to indicate that it is allowed, even if not preferred.

with the only other definition/clarification of 'integer' in the spec (on the instance data model) stating that it allows a 0 fractional part, and nothing indicating an integer should mean something different in a schema than an instance, it seems reasonable to me to expect it to be allowed.

given that the spec explicitly changed from previous drafts disallowing integers a zero fractional part in the instance data model, I think if it were intended that an integer in a schema should mean something different, that language would have been added.

[Relequestual]

the fact that the language is a SHOULD NOT instead of a MUST NOT in core 6.3...

Right, I'm not saying it IS NOT allowed, but more that it isn't REQUIRED, and so should not be in the default required tests.

I think if it were intended that an integer in a schema should mean something different, that language would have been added.

That isn't the way specifications work. We don't actually define an "interger" type for schemas, only instances.
In terms of schema processing, numbers are numbers are numbers, as per the JSON specifciation.

I don't have any problem adding these tests, but they MUST be optional.

[Relequestual]

Because the specification only defines that number and integers with zero fractional in terms of instance data model and evaluation, not schema processing, these tests need to be optional, and not required.

[notEthan]

given core 6.3, it seems like a stretch to call this optional. if integers in schemas are allowed by "SHOULD NOT be encoded with a fractional part" to have that fractional part, validators should be expected to support that valid way of expressing a schema.

why would the spec change both the instance data model and the general section on mathematical integers to allow (albeit disapprovingly) a fractional part, but not have them be a supported thing for schemas? I know looking at this history "isn't how specs work", but it is context that sure makes it look like supporting this is intended.

there's a tiny sliver of plausibility that it's not supposed to be explicitly allowed, in the space between schemas as objects defined by the spec and as instances described by the metaschema. core 6.3 still applies there, but it is a SHOULD NOT, so maybe validators don't have to treat integers in schemas that way.

if this is optional, it's not really worth testing. I do believe I've stopped caring about this PR.

[Relequestual]

Looks like we need to re-read and evaluate json-schema-org/json-schema-spec#898

6.3 does not related to allowable values in a schema.
As #898 comments point out, that section is related to the encoding of JSON, which is likely out of scope of JSON Schema in the first place.

I do remember something about mathmatical equivalency though, but I don't know where that is now.

[ssilverman]

@notEthan I’d still love to see these tests here. If an implementation has had trouble with something, I agree with your approach to write tests for it. What do people think about reopening?

[ssilverman]

@notEthan @Relequestual one more comment: I believe these tests test expected behaviour and should be required.

[Relequestual]

I think we need someone else to weigh in on it. *looks at @awwright @handrews *

[awwright]

Schemas are technically instances, and are evaluated with the same data model where 1.0 == 1. (Otherwise, how could you evaluate a schema against the meta-schema?) This looks correct to me.

[Relequestual]

I'm not dissageeing with the fact that schemas can be instances if a meta-schema is applied to them. That isn't related to the issue here. The question is, for values which are defined in the meta-schema as integers, should implementations be required to support said values with a zero fractional part?

A note at json-schema-org/json-schema-spec#898 (comment) ...

thanks- to clear up possible confusion, I think I focused on the wrong direction (reading in numbers). This clarifies that it's about writing them out, as this ensures that a C float 10, a C int 1 (different types in C), and a JavaScript number 1 or 1.0 (same type in JavaScript) produce the same representation in JSON.

My one question is: Is JSON encoding even within the scope of our spec?
- @handrews

Is JSON encoding even within the scope of our spec?

You're right, it's not. I no longer think a capitals SHOULD is appropriate here.

We could probably take out the entire section, or significantly rewrite it.
- @awwright

ORIGINALLY the issue we wanted to resolve is recognising that the processing model of JSON Schema should make allowances for where languages would find it very problematic to distinguish between 1 and 1.0, and povide a get out clause for those situations. The intent was not to require implementations which can distingusih between, to require supporting zero fractional parts in places where an integer is expected.

For example, it doesn't ever make sense for maxLength to have a fractional part, so the meta-schema says the value is a non negative integer. Now, 1.0 would be valid according to the meta-schema, but that shouldn't mean the implementation must be required to support 1.0 as a valid value for that property.

---

As an aside, I noticed that we have sometimes use "non negative integer" and sometimes just number (where it should also be non negative integer). I'm assumign that's an oversight. If you concur, a new issue needs to be filed.

https://github.com/json-schema-org/json-schema-spec/blob/9b3294696b331790ca4e39b1e8ff8ccdc651ad7d/meta/validation.json#L22-L29

[Relequestual]

To clarify, I'm not saying that we CAN'T make this the case in the spec, but I am saying the 2019-09 draft and the current in progress draft do NOT make this a requirement.

[notEthan]

I'll address the aside first: the only place I see 'number' in the metaschema, including your link highlighting minimum, it applies to mathematical operator keywords that definitely should include non-integers. the non-negative integer keywords deal with counting things.

[notEthan]

I thank you for the time and thought you have put into clarifying this, @Relequestual. I haven't read all the historical discussion of integers in the linked issue, so I'm content to defer to your understanding of all that, and your conclusion that this is optional.

it seems to me that logically implementations should have the same rules on a thing like this for schemas and their instances; I think the considerations around encoding and internal representations are very similar or identical for both. I think it might be an improvement if the spec were to align that, but I don't plan to pursue that myself.

6.3 does not related to allowable values in a schema.

I'm hesitant to raise this - I am content to let this rest and don't mean this to be argumentative, so please feel free to leave this without response. but I am confused why the section "general considerations" (6) would not apply to the values in a schema. the rest of section 6 is a mix of considerations which apply to schemas and instances.

[Relequestual]

I'll address the aside first: the only place I see 'number' in the metaschema, including your link highlighting minimum, it applies to mathematical operator keywords that definitely should include non-integers. the non-negative integer keywords deal with counting things.

Ah oops, that's me pre-coffee forgetting that THAT keyword is not minContains... Lemme just ---- that... =D

[Relequestual]

6.3 does not related to allowable values in a schema.

... I am confused why the section "general considerations" (6) would not apply to the values in a schema. the rest of section 6 is a mix of considerations which apply to schemas and instances.

Reviewing this, I may be inclined to agree.
I'm going to review the comments in this PR, again... =]
I still think that json-schema-org/json-schema-spec#898 is related though.

[notEthan]

@Relequestual do you still object to this PR? I'll rebase if you don't; if you still do you can close it.

[Julian]

I think by the end here that Ben you're at least coming around to agree with the PR?

Essentially the clause that a lot of this PR went back and forth on is about writing out JSON, not reading it -- whether we have the "right" to even govern how to write out JSON aside, for reading, JSON doesn't distinguish between 1.0 and 1, they're both just numbers, and given we say "1.0 and 1 are both integers as far as the specification is concerned", it doesn't seem like a leap to say that's the case in schema values either -- yes, you can read them in as floats (which the JSON spec allows for) but then you should treat them as integers for any JSON Schema validators that want integers. At least that's my interpretation.

Raise any objection if you have one, but given Austin seems to have thumbs-uped this, and to me it seems correct as well, and your last comment seems to say you're agreeing, going to hit the button later today or tomorrow unless you or someone else speaks up. Sorry this took so long @notEthan.

[notEthan]

sounds good. I've rebased against main.

[notEthan]

and added draft2020-12

[Julian]

Thanks! Can you add it to draft-next as well, and then should be good to go!

[notEthan]

added draft-next, squashed, and another rebase from main

[Julian]

Thanks!