Skip to content
This repository has been archived by the owner on Nov 3, 2023. It is now read-only.

Truth table for conditional subschemas is incomplete #117

Closed
HertzDevil opened this issue Mar 3, 2020 · 15 comments
Closed

Truth table for conditional subschemas is incomplete #117

HertzDevil opened this issue Mar 3, 2020 · 15 comments

Comments

@HertzDevil
Copy link

The truth table on the Applying subschemas conditionally section seems to be missing the rows corresponding to the cases when if is invalid. I believe the full table should be like this:

if then else whole schema
 
X X
X
X X X
X
X X
X X X
X X X X
@notEthan
Copy link

notEthan commented Mar 3, 2020

the choice of X / blank to indicate what's valid is pretty unintuitive to me. the book doesn't even explain "X means valid" (I initially assumed X meant failure). I've applied the magic of unicode to help make this more intuitive-looking. I've also ordered things where valid schemas go first and thrown in explanatory comments.

if then else whole schema
`if` schema validates; `then` schema result applies to the instance. `else` is not applied.
`if` schema fails to validate; `else` schema result applies to the instance. `then` is not applied.

@notEthan
Copy link

notEthan commented Mar 3, 2020

the source, since github doesn't make that easy to see:

<table>
  <tr>
    <td>if</th>
    <td>then</th>
    <td>else</th>
    <td></th>
    <td>whole schema</th>
  </tr>
  <tr>
    <td colspan=5>`if` schema validates; `then` schema result applies to the instance. `else` is not applied.</td>
  </tr>
  <tr>
    <td></td>
    <td></td>
    <td></td>
    <td></td>
    <td></td>
  </tr>
  <tr>
    <td></td>
    <td></td>
    <td></td>
    <td></td>
    <td></td>
  </tr>
  <tr>
    <td></td>
    <td></td>
    <td></td>
    <td></td>
    <td></td>
  </tr>
  <tr>
    <td></td>
    <td></td>
    <td></td>
    <td></td>
    <td></td>
  </tr>
  <tr>
    <td colspan=5>`if` schema fails to validate; `else` schema result applies to the instance. `then` is not applied.</td>
  </tr>
  <tr>
    <td></td>
    <td></td>
    <td></td>
    <td></td>
    <td></td>
  </tr>
  <tr>
    <td></td>
    <td></td>
    <td></td>
    <td></td>
    <td></td>
  </tr>
  <tr>
    <td></td>
    <td></td>
    <td></td>
    <td></td>
    <td></td>
  </tr>
  <tr>
    <td></td>
    <td></td>
    <td></td>
    <td></td>
    <td></td>
  </tr>
</table>

@mdboom
Copy link
Member

mdboom commented Mar 11, 2020

Any interest in turning this into a pull request?

@ghost
Copy link

ghost commented May 22, 2020

I was just looking into this myself and was going to do a PR. I don't know if @notEthan or @HertzDevil are still interested. Looking at the source it looks like this has almost been completely addressed (without the unicode characters, and missing 1 row), but perhaps it's just not been published.

Here's what the source currently looks like:

==== ==== ==== ============
if   then else whole schema
==== ==== ==== ============
          X    X
     X          
     X    X    X
X               
X         X     
X    X         X
X    X    X    X
==== ==== ==== ============

The only suggestion I'd make is to put the first line in. I'll submit a PR with this completed and with the unicode.

@dima1206
Copy link

There is also a mistake before the table: "If if is invalid, else must also be valid". I guess it should be "If if is invalid, else must be valid" (removed "also").

About the table. It can be reduced using "-" or something similar, so it would be like this:

if | then | else || whole schema
--------------------------------
❌ |  --  |  ❌  ||    ❌
❌ |  --  |  ✅  ||    ✅
✅ |  ❌  |  --  ||    ❌
✅ |  ✅  |  --  ||    ✅

@karenetheridge
Copy link
Member

I like reducing the cases together to show that the results of the other branch (of "then" and "else") don't matter. Indeed, the implementation won't even run them, which is a distinction that should be made clear to the user.

@dima1206
Copy link

Just remembered, that when I was reading, I also was wondering about omitting else. Seeing example at the end of the page, I guess that it works the same as {} schema. Here is one more variation of the table (not sure about then):

if | then  | else  || whole schema
--------------------------------
❌ |  --   |  ❌   ||    ❌
❌ |  --   |  ✅   ||    ✅
❌ |  --   |omitted||    ✅
✅ |  ❌   |  --   ||    ❌
✅ |  ✅   |  --   ||    ✅
✅ |omitted|  --   ||    ✅/incorrect schema

Maybe also add some explanation about omitting.

@notEthan
Copy link

thanks for picking this up, @mgwelch.

I do agree it's simpler with the unused control paths collapsed, but not strongly opinionated about it.

@ghost
Copy link

ghost commented May 26, 2020

I do like the collapsed as well. I can make the change. Adding omitted is very important as well. I didn't even realize that was a valid option.

@ghost
Copy link

ghost commented May 26, 2020

Here is one more variation of the table (not sure about then):

Wouldn't omitting then work just like else? It seems your table is correct, I just didn't understand your trailing comment: "/incorrect schema"

@ghost
Copy link

ghost commented May 26, 2020

Just remembered, that when I was reading, I also was wondering about omitting else. Seeing example at the end of the page, I guess that it works the same as {}

It might keep the table simpler to NOT call out the omitted cases but instead mention (if true) that an omitted schema is always valid since it works the same as {}.

The reason to not call it out in the table is that it gets complicated representing both "don't care" cases and omitted cases. Or at least I think it gets more complicated to see a boolean truth table with 4 different entries in the cells.

Perhaps we could put a footnote on the ✅ cases for then and else indicating that it includes the omitted cases.

@dima1206
Copy link

dima1206 commented May 26, 2020

I just didn't understand your trailing comment: "/incorrect schema"

As I said, I'm not sure about omitting then, so depending on that, there should be either '✅' or 'incorrect schema'. I guess(!) that it should be the first one. JSON schemas are new to me so don't take my words as truth.

Maybe you're right that omitting shouldn't complicate the table, and adding footnote would be enough.

@ghost
Copy link

ghost commented May 26, 2020

As I said, I'm not sure about omitting then

The spec I guess is clear enough (it took me a few times to read it to understand that it is clear):

if

This keyword's value MUST be a valid JSON Schema.

This validation outcome of this keyword's subschema has no direct
effect on the overall validation result.  Rather, it controls which
of the "then" or "else" keywords are evaluated.

Instances that successfully validate against this keyword's subschema
MUST also be valid against the subschema value of the "then" keyword,
if present.

Not that it's guaranteed to be a correct implementation, but I did confirm with ajv that this instance

{
    "id": 0,
    "name": "Michael"
}

conforms to this schema

{
  "if": {
    "properties": {
      "id": { "const": 0 }
    }
  },
  "else": {
    "properties": {
      "name": {
        "type": "number"
      }
    }
  }
}

@ghost
Copy link

ghost commented May 27, 2020

@mdboom I've submitted a PR but there are issues with unicode characters and Latex. In the PR (#127) I post a screen shot of an alternative.

@jdesrosiers
Copy link
Member

Fixed by #143

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

No branches or pull requests

6 participants