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

Tighten up wording of 'Default values for missing keywords' #4

Closed
wants to merge 1 commit into from

Conversation

pdl
Copy link
Contributor

@pdl pdl commented Jul 8, 2015

I believe the intention of the previous wording here was that
implementations could represent default values by changing the
in-memory representation of the schema so that the default values are
present, rather than by changing the code which acts on the in-memory
representation to follow the default behaviour each time.

However, the wording was also open to other interpretations, e.g. that
implementations could ignore default values entirely (and might
therefore cause errors or behave in the opposite manner if the default
value is boolean true).

Since there is nothing in this schema which would be affected by such
changes to the in-memory representation, it can be assumed that this
representation is entirely within the domain of the implementation and
there is no need to explicitly permit inference of defaults; the only
real requirement is that the result be the same.

I believe the intention of the previous wording here was that
implementations could represent default values by changing the
in-memory representation of the schema so that the default values are
present, rather than by changing the code which acts on the in-memory
representation to follow the default behaviour each time.

However, the wording was also open to other interpretations, e.g. that
implementations could ignore default values entirely (and might
therefore cause errors or behave in the opposite manner if the default
value is boolean true).

Since there is nothing in this schema which would be affected by such
changes to  the in-memory representation, it can be assumed that this
representation is entirely within the domain of the implementation and
there is no need to explicitly permit inference of defaults; the only
real requirement is that the result be the same.
@awwright
Copy link
Member

awwright commented Jul 9, 2015

The use of the caps "MAY" imposes an operational requirement, here it simply seems to be saying "if the keyword is allowed to... then..." which isn't an implementation/operational requirement as such. So maybe make it lowercase?

I've found the best use of the "default" keyword is providing a good default value for use in producing new instances of the schema.

This is as opposed to saying "if this value is absent, it will effectively be the same as the value provided in default"

Though in many cases this might still be a logical consequence of my definition. For instance, if I add a property to an object (like "checked" on a checkbox), it should default to the value that expresses the state of the checkbox if no property existed (presumably, checked=false).

@jdesrosiers
Copy link
Member

@pdl, I found the new wording a bit difficult to digest, but I don't think this change is necessary. All of the keyword default values define the no-op behavior for that keyword. An implementation that ignores defaults would do nothing, while an implementation that doesn't ignore defaults would execute a no-op. Therefore, there it is not necessary to explicitly state that validators MUST behave the same whether the keyword is absent or present with the default value.

I wouldn't change the current wording, but I would support adding a few sentences to the spec stating that the default value of a keyword is equivalent to it's no-op case. This should be sufficient to mitigate the misinterpretations.

@pdl
Copy link
Contributor Author

pdl commented Jul 10, 2015

Some keywords, if absent, MAY be regarded by implementations as having
a default value. In this case, the default value will be mentioned.

The problems I have with the current wording are:

Firstly, it is unclear what case is referred to by "In this case":

  • In the case of some keywords?
  • In the case of their absence?
  • In the case that they may be regarded as having a default value?
  • In the case that they are in fact regarded as having a default value?

It's probable that there is only one possible meaningful interpretation, but the readability is compromised by the unclear grammatical relationship which is jarring.

Secondly, the order is strange for a spec. Usually we say: "In situation X, implementations may/must/should/ do Y". Here it reads backwards "Implementations may sometimes do Y. If Y can be done, the spec will say so."

These two features combine to make me feel as if I don't understand what it's saying when (I think) it's saying something relatively trivial.

If it can be reworded I don't object to the content, but as it stands it seems to me that it will do more harm than good.

The reasons for the addition are:

  1. Because it is easier to define a default than to define a default and confirm that the no-op behaviour is the same as the behaviour with the default value each time;

  2. Because if stated once globally it's less likely that if future changes accidentally omit a default behaviour it will make a difference

  3. Because a later part of the spec also states:

    In addition, it is important that if one or more keyword(s) implied in the
    calculation are not defined, they be considered present with their default
    value, which will be recalled in this section.

    And we may as well just do this once at the top of the spec rather than giving the impression this is a different case.

@jdesrosiers
Copy link
Member

Ok, I agree that the "In this case" wording is awkward. In fact the whole second sentence is awkward and could be improved. But, I didn't have any trouble figuring out it's meaning.

Where keywords are specified which MAY be absent but which have a default value described, implementations MUST produce the same results whether the keyword is absent or whether it is present and has the default value.

I find the new wording much more difficult to understand. Even reading slowly multiple times, I'm still not entirely sure what the first half of this sentence means. I like the second half of the sentence. It is effectively equivalent to what I was saying in my previous comment. But, I wouldn't remove the original wording. I would add it as a clarification.

The second part of the spec you quoted is the part that I would change. It is in fact not important that keywords be considered present with their default value. It is no different than omitting that part of calculation. This part of the spec actually makes JSON Schema no longer self describable. JSON Schema has no keywords that allow us to specify a default value that must be considered present if omitted. The default keyword is a meta-data keyword and has no meaning in validation. If the specification does not set this unnecessary requirement, the JSON Schema meta schema fully describes a JSON Schema.

@Relequestual
Copy link
Member

I think the abiguity here is in the capitalisation of "MAY". I can't believe the intension would be to allow optional consideration of a default value. Re-wording is needed here. I would consider a slight alteration on @pdl to

Where keywords are specified which may be absent, and have a default value described, implementations MUST treat an absent keyword's value as if it had been set to the default value.

@ralfhandl
Copy link

I like the shorter wording proposed by @Relequestual

@awwright
Copy link
Member

I modified the wording of the section in question in commit 11a0063. Comments welcome!

@Relequestual
Copy link
Member

Relequestual commented Sep 19, 2016

@awwright I'm worried by your commits directly to master! People coming to the github will believe what is in master is the current release. I would feel eased if you re-phrase the wording in the readme welcome section to explain this repo contains "work in progress for new versions, and previous versions". Further, I'm keen to have a policy of always making pull requests which must be accepted by someone else, and never directly comitting to master.

@awwright
Copy link
Member

@Relequestual Sorry for that! Unfortunately there's not too many people with write access (three?), I'm trying my best to atopt the IETF "working code and rough consensus" model, and get a personal I-D out before the f2f deadline at the end of this month.

I think I misunderstood the original issue here, in my original comment. When rewriting the paragraph more recently I ended up taking "default" out of the wording entirely because the draft isn't really specifying anything like defaults at all.

Only a handful of keyword definitions actually used the phrase. But really what we mean is "hey, this value is the same behavior as not existing".

As far as I understand, this issue suggests adding the requirement "Certain values MUST have the same behavior as not being specified", which we can just write into the respective keyword definitions.

So this is still all open to consideration!

@awwright
Copy link
Member

As far as I can tell, the latest rewrite should answer some of the questions that were posed.

If it doesn't open a new issue and we'll see if we can fix whatever is unclear!

@awwright awwright closed this Oct 13, 2016
handrews added a commit that referenced this pull request Nov 16, 2017
replace href* keywords by template* in collections example
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

Successfully merging this pull request may close these issues.

5 participants