Skip to content

Validation: Replace "considered present" language #171

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

Conversation

awwright
Copy link
Member

@awwright awwright commented Nov 30, 2016

This was an issue for #131

@handrews
Copy link
Contributor

This offers less guidance for implementation and does not line up with the usage of the "default" keyword in the meta-schema. I much prefer the existing language, which lets me know that I can safely substitute values and then ignore whether they came from defaults or not. While the meta-schema does convey this with "default", you have made it clear numerous times that the meta-schemas are non-normative, so this language is important.

@Relequestual
Copy link
Member

A much simpler fix for the issue mentioned would be to change can be to must be

@awwright
Copy link
Member Author

awwright commented Dec 1, 2016

@handrews Medata keywords are doing different jobs than validation keywords. Validation keywords don't have defaults, they simply don't exist if they're not specified. This is true for some metadata keywords, too.

The remaining metadata keywords do things like switch a flag - either an instance is readOnly or it isn't. These can be said to have default values.

Perhaps related to this issue is that the "default" keyword is somewhat confusingly defined.

@Relequestual I'm wary of that language, that sounds like normative language, which would be redundant since that's something that's already going to be true anyways. And actually, part of my distaste for the current language is that it sounds normative.

@handrews
Copy link
Contributor

handrews commented Dec 1, 2016

@awwright I don't buy it. The previous wording very clearly aligned with how "default" works, and with how the meta-schema is written. I see no evidence for the view that you assert, no benefit, and considerable drawbacks. You're going to need to go into a lot more detail about precedent and benefits to sell me on this.

There are indeed keywords that cannot be defaulted, and when missing are just missing, but those are not the ones that you changed.

@awwright
Copy link
Member Author

awwright commented Dec 1, 2016

@handrews Alternatively, we can remove the language in question. The language is supposed to be informative and the language has confused at least one person, do you have a better alternative?

@handrews
Copy link
Contributor

handrews commented Dec 1, 2016

On IRC @awwright floated the language "defaults to a value of", which I like a lot.

However, he went on to say:

I guess I think it's misleading to one's comprehension. Validators have a list of constraints, either a constraint exists or it doesn't, there's no concept of existing before it was defined, so there's no default value to exist with

I would like to know more about where this philosophical view of non-existing keywords comes from, as I don't follow the "no default value to exist with" conclusion.

@@ -331,7 +331,7 @@
Each object MUST be a valid JSON Schema.
</t>
<t>
If absent, it can be considered present with an empty schema.
A missing keyword is the same as an empty schema.
</t>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New wording is more ambiguous than the old.

@@ -393,7 +393,7 @@
greater than, or equal to, the value of this keyword.
</t>
<t>
If this keyword is not present, it may be considered present with a
A missing keyword is the same as a
value of 0.
</t>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The old language is clearer. In the new it is not clear a value of what.

@awwright awwright closed this Dec 11, 2016
@awwright awwright deleted the considered-present-considered-harmful branch December 11, 2016 19:32
@awwright awwright restored the considered-present-considered-harmful branch December 11, 2016 19:33
@awwright awwright reopened this Dec 11, 2016
@handrews
Copy link
Contributor

I'd prefer "behaves the same as a value of..."
but this version is a step in the right direction, pending @epoberezkin's reaction.

@epoberezkin
Copy link
Member

epoberezkin commented Dec 11, 2016

How about "An absent keyword is equivalent to the keyword with the value 0"?

@awwright
Copy link
Member Author

@epoberezkin That suggestion is sort of what I'm getting at with this, that sounds good.

Also, the more I keep thinking about this, I think I'm more comfortable with using the term "Default". I just want to make it clear, if there's a default value, that doesn't imply it exists even if undefined - default just means a suitable initial value, if and when an instance is created or property/keyword is defined. Admittedly, this is less of an issue for validation keywords.

@awwright awwright assigned awwright and unassigned Relequestual Dec 13, 2016
@handrews
Copy link
Contributor

that doesn't imply it exists even if undefined - default just means a suitable initial value, if and when an instance is created or property/keyword is defined.

I still have no idea what you're getting at with this. What meaningful distinction is present here?

@Relequestual
Copy link
Member

I'd like to see a MUST keyword added to this phrasing to avoid any question over if this is an optional thing to do or not. After all, that's what #131 raised confusion over.

@awwright
Copy link
Member Author

@handrews In my understanding and use, even if I say default: null about a property, that doesn't necessarily imply that {value: null} will have the same behavior as {}. All it means is if I say "I'm adding a property called 'value'" -- then you can give it the default value, since you have to create it with something.

@Relequestual The use of "MUST" adds a behavioral requirement to implementations (or modifies what is considered a valid document). What behavior would that be, exactly? The existing language is supposed to be strictly informative - it's saying "hey, please note that this behaves the same as that" which is useful to assist comprehension.

@handrews
Copy link
Contributor

@awwright I think you are making default harder to grasp than it really is. What is the benefit of insisting that {value: null} and {} can behave differently when the value property has a default of null? And where is the requirement stated that such a difference be allowed?

@handrews
Copy link
Contributor

@awwright @Relequestual I feel like we should make the language for "default" more clear until we're comfortable with just saying that these are defaults. If we're avoiding the term "default" here because it is confusing or doesn't indicate the correct thing, that's a problem for schema authors in general that we should address.

@Relequestual
Copy link
Member

I don't think anyone is clear about a few things with this PR / related issue.

  • It's unclear what the current phrasing means
  • It's unclear what we WANT to the phrasing to actually mean

@awwright You commented that MUST adds a behavioural requirement. Indeed. I think the wording currently is ambigious and means interoperablity is potentially broken here.

Consider:
"If you loose the bet, you're considered to owe me money"
vs
"If you loose the bet, you MUST give me the money owed"

I'd say that's a behavioural change which removes ambiguity and makes expected behaviour better defined.

The issues you reference for that PR (#131): The user has questions regarding expected behaviour. Changing the phrasing doesn't FIX that unless it perscibes behaviour rather than making it a stronger suggestion.

@Relequestual
Copy link
Member

I'd like to suggest we open a new issue for this PR, as the original issue is now closed, and the problem statement is not clear, so it's difficult to work out what this PR intends to fix.

@awwright
Copy link
Member Author

@Relequestual This is an editorial change, as opposed to behavioral (design) one. I don't think there was an original issue.

This might be a good part of a larger issue to rephrase the validation language, though. It has a few clarity issues and it might be under-constrained. Lemme look more into it.

@handrews
Copy link
Contributor

handrews commented Jan 5, 2017

@awwright @Relequestual I think that there are a lot of small but significant issues swirling around both the language for default and for other annotation keywords (title and description). Relevant issues include:

#136 General annotation/documentation discussion and proposals
#204 Extensive discussion of "default", leading to...
#217 Proposal for a hyper-schema-specific "messageDefault"

I agree with @Relequestual that the wording for "default" (and other annotation keywords) should be made less ambiguous. I also agree that it would be best to close this, continue discussion in the issues mentioned above, and try again with a PR when the ideas are more clear.

@Relequestual
Copy link
Member

After re-reading this PR, and @awwright's explanation at #131 (comment), I feel I understand what @awwright is getting at in this PR.

The laguage used isn't to define behaviour, but it doesn't give validator writers anything to do.

I guess you can think about them as default values, but strictly speaking they're just different values that exhibit the same behavior (never fails to validate).
( @awwright )

The orignal issue quoted...

If "additionalProperties" is absent, it may be considered present with an empty schema as a value.

What we actually want to say is, {additionalProperties: {}} is the same as not including additionalProperties in the first place. Omitting additionalPropertiesdoesn't indicate any validation behaviour, and adding it with an empty schema also doesn't indicate any validation behaviour. I just can't think of a good way to phrase that.

Probably @epoberezkin's suggestion of the following earlier in this PR makes the most sense...

"An absent keyword is equivalent to the keyword with the value 0"

I feel the best cause of action is to either fix this PR to match that, or open an new PR and close this one.

@handrews
Copy link
Contributor

@Relequestual @awwright I still think that the underlying problem here is that "default" is under-specified for this use. There is a simple question here: Is it or is it not reasonable to write the default value into an instance during processing. It is definitely not required, and I'd say MUST NOT be expected, but is it forbidden? Is it at the discretion of the app (in this case the app is a validator)? Is it at the discretion of the user of the app?

As an example: in the hyper-schema client that I'm working on, when you examine a property or element in an instance, it figures out what schema or schemas apply so that the hyper-client part of the system can tell whether there are any links defined. If the property is not covered by a keyword, I will return an empty schema (the default for "additionalProperties"). That is more convenient than raising an error or having a bunch of special-case handling for a null value.

I really feel that the wording problem here is a symptom and not the fundamental concern. If we clarify what can and can't be done with "default" (using proper MUST/SHOULD/MAY language), or provide an additional keyword that is more narrowly defined than "default" so that it can require specific behavior, then simply saying that these are defaults (or whatever the other keyword is) will be unambiguous.

@handrews
Copy link
Contributor

@awwright clearly there's a lot of difference of opinion here and nothing resembling consensus. Should we punt this out to a future draft, or do you want to hold Draft 06 for this?

@awwright
Copy link
Member Author

I'll publish a revision tomorrow

@awwright awwright closed this Jan 27, 2017
@Relequestual
Copy link
Member

@awwright Great! Please make sure to add the draft-6 milestone to that new PR

@awwright awwright deleted the considered-present-considered-harmful branch February 1, 2017 15:56
@gregsdennis gregsdennis added clarification Items that need to be clarified in the specification and removed Type: Maintenance labels Jul 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clarification Items that need to be clarified in the specification Priority: Low
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants