Lazy eval version #170

gkellogg · 2019-10-16T22:48:19Z

Updates to existing tests to make json-ld-1.1 the presumed version, unless json-ld-1.0 is set explicitly.
Don't explicitly set processingMode to 1.1 in tests, as it's now implicit.

* Updates to existing tests to make json-ld-1.1 the presumed version, unless json-ld-1.0 is set explicitly. For #161.

…icit.

dlongley · 2019-10-17T15:33:16Z

index.html

+    in a local <a>context</a> and
+    affects the behavior of algorithms including <a>expansion</a> and <a>compaction</a>.
+    Once set, it is an error to attempt to change to a different processing mode,
+    and processors MUST generate,


What if we want, in the future, to reuse a context with "@version": 1.1 within a context that with "@version": 1.2? It seems like a reasonable future scenario. Wouldn't this cause an error? Am I misinterpreting it or is that intentional ... or something else? Maybe this should say you can't downgrade but you can upgrade?

"you can't downgrade but you can upgrade?" sounds like a reasonable statement to me...

I agree with @iherman. NB: if this is changed, this should be changed accordingly in the Framing doc.

Agreed, you can upgrade; for 1.1, it would be an error to use 1.2, just as with 1.0, it's an error to use 1.1.

That said, it should never be necessary to detect things based on @version in the future, as we specifically check for things we expect in context processing, and reject anything else. We didn't do this properly in 1.0.

dlongley · 2019-10-17T15:37:17Z

index.html

                  a <a data-link-for="JsonLdErrorCode">colliding keywords</a>
                  error has been detected and processing is aborted.</li>
+                <li class="changed">If <var>result</var> has already an <var>expanded property</var> <a>entry</a>,
+                  for `@included` or `@type`.


It seems like the above period should either be a comma or we're missing the "if json-ld-1.0 mode is set throw an error" bit.

The previous list entry should cause the colliding keywords error to be thrown, so that will already have happened.

pchampin

In many places, the spec now says:

If processing mode is json-ld-1.0, an XXX has been detected and processing is aborted. Set processing mode, to json-ld-1.1, if not already set.

First the comma after "processing mode" is strange.. I suggest to remove it.

More importantly, this phrasing can be misleading: the second sentence ("Set processing mode...") is subordinated to the "If" at the start of the first sentence.

I suggest rephrasing this to

If processing mode is json-ld-1.0, an XXX has been detected and processing is aborted. Otherwise, set processing mode to json-ld-1.1, if not already set.

or moving the second sentence ("Set the processing mode...") to a new <li>.

If this proposal is agreed on, I can push a commit to that effect.

pchampin · 2019-10-17T18:45:09Z

index.html

              If the lexical value of <var>value</var> is not valid JSON according to
              the <a data-cite="RFC8259#section-2">JSON Grammar</a> [[RFC8259]],
              an <a data-link-for="JsonLdErrorCode">invalid JSON literal</a>
              error has been detected and processing is aborted.</li>
-            <li class="changed">Otherwise, if <a>processing mode</a> is `json-ld-1.1`,
-              the <a>datatype IRI</a> of <var>value</var> starts with `https://www.w3.org/ns/i18n#`,
+            <li class="changed">Otherwise, if the <a>datatype IRI</a> of <var>value</var> starts with `https://www.w3.org/ns/i18n#`,


If processing mode is explicitly json-ld-1.0, I would expect this kind of datatype to be kept as is...
So the condition would become

if <a>processing mode</a> is not `json-ld-1.0` and the <a>datatype IRI</a>...

It shouldn't be necessary, as the statement is already predicated on the rdfDirection option, which an only appears in 1.1.

gkellogg · 2019-10-17T20:40:15Z

index.html

-        the <a>processing mode</a> is set using the <code>@version</code> <a>entry</a>
-        in a local <a>context</a> and
-        affects the behavior of algorithms including <a>expansion</a> and <a>compaction</a>.</p>
-


Note that what we've effectively done is have a 1.0 mode; setting @version to 1.1 only serves to stop things which have configured to run in 1.0 mode. All the places we say to update processingMode to json-ld-1.1, if necessary, don't actually have any effect.

gkellogg · 2019-10-17T20:40:54Z

index.html

                  an <a data-link-for="JsonLdErrorCode">invalid context entry</a>
-                  error has been detected and processing is aborted.</li>
+                  error has been detected and processing is aborted.
+                  Set <a>processing mode</a>, to `json-ld-1.1`, if not already set.</li>


Removing this sentence, and others like it, really would make no difference in how we actually process.

gkellogg · 2019-10-17T20:52:29Z

In many places, the spec now says:

If processing mode is json-ld-1.0, an XXX has been detected and processing is aborted. Set processing mode, to json-ld-1.1, if not already set.

First the comma after "processing mode" is strange.. I suggest to remove it.

👍

More importantly, this phrasing can be misleading: the second sentence ("Set processing mode...") is subordinated to the "If" at the start of the first sentence.

I suggest rephrasing this to

If processing mode is json-ld-1.0, an XXX has been detected and processing is aborted. Otherwise, set processing mode to json-ld-1.1, if not already set.

or moving the second sentence ("Set the processing mode...") to a new <li>.

If this proposal is agreed on, I can push a commit to that effect.

See in other comments where I suggested removing all of the "Set the processing mode..." clauses, as they really have no effect.

I'll take care of that if people are in agreement. Note, however, that this goes against the "lazy evaluation of 1.1 mode" we had discussed, and is defaults to 1.1, and allows to guard against 1.1 changes to setting processingMode to json-ld-1.0 in the options.

gkellogg · 2019-10-17T20:55:54Z

In many places, the spec now says:

If processing mode is json-ld-1.0, an XXX has been detected and processing is aborted. Set processing mode, to json-ld-1.1, if not already set.

First the comma after "processing mode" is strange.. I suggest to remove it.

Actually, looking at it again, it's a fairly long-established practice, and in english, these are different clauses, "If CONDITION, then RESPONSE". It is also a natural place to pause when speaking.

iherman · 2019-10-18T17:23:00Z

This issue was discussed in a meeting.

RESOLVED: Accept changes after changing clause for explicit 1.1 for API #161, lazy evaluation of processing mode

View the transcript

Lazy evaluation of 1.1 processing mode
Rob Sanderson: #161
Gregg Kellogg: Also w3c/json-ld-syntax#284
Gregg Kellogg: w3c/json-ld-framing#76
Gregg Kellogg: #170
Rob Sanderson: It would be easier for version migration compliance to handle @version keyword lazily, and let processors detect them based on the features that are being used.
Gregg Kellogg: When we discussed it, the idea was that we would do feature detection, and move up to 1.1 when we saw that.
… If you are explicitly in 1.0 mode, then you don’t run any 1.1 features. If one such feature would be 1.1, then a 1.0 processor would terminate.
… This would happen in any case on old processors if they see unknown (new) features.
… The PRs describe this slight change and the necessary steps.
Dave Longley: +1 to gregg’s description of how lazy eval works
Rob Sanderson: There was a question about the tests to see if there were any issues.
Gregg Kellogg: There weren’t any exceptions. I just added a couple of tests.
… I’ve updated many tests that used to the processing mode explicitly, and things seem to work correctly.
Dave Longley: As we will probably will have a JSON-LD 1.2 at some point, we don’t want to lock down the version in the algorithm.
Ivan Herman: +1 dlongley
Gregg Kellogg: I agree.
Rob Sanderson: +1 as well
Rob Sanderson: Is this written like that in the PRs?
Dave Longley: +1 to remove those clauses
Gregg Kellogg: Yes, the PRs make it so that processing mode can be “unset”, which allows the silent upgrade.
Dave Longley: +1 to merge the PRs with the above changes
Ivan Herman: +1 for me, too
Gregg Kellogg: The conformance section describes changes to proc mode as well, besides the algorithmic changes.
Proposed resolution: Accept changes after changing clause for explicit 1.1 for API #161, lazy evaluation of processing mode (Rob Sanderson)
Rob Sanderson: +1
Gregg Kellogg: +1
Ruben Taelman: +1
Dave Longley: +1
David I. Lehn: +1
Pierre-Antoine Champin: +1
Resolution #3: Accept changes after changing clause for explicit 1.1 for API #161, lazy evaluation of processing mode
Gregg Kellogg: Another useful thing would be a doc/post about the process to update to 1.1.
Ivan Herman: We have to clarify that this is not just lazy evaluation, but it is better than just that.

gkellogg added 3 commits October 16, 2019 12:33

Lazy-Eval processingMode.

db81e26

* Updates to existing tests to make json-ld-1.1 the presumed version, unless json-ld-1.0 is set explicitly. For #161.

Update spec for lazy-evaluation of processingMode.

9dd31b3

Don't explicitly set processingMode to 1.1 in tests, as it's now impl…

f3be181

…icit.

gkellogg requested review from davidlehn, dlongley and pchampin October 16, 2019 22:48

Change note.

7cbd6b0

dlongley reviewed Oct 17, 2019

View reviewed changes

pchampin requested changes Oct 17, 2019

View reviewed changes

gkellogg commented Oct 17, 2019

View reviewed changes

"has already an" => "already has an".

ff8e556

Update terms.

2029336

This was referenced Oct 18, 2019

lazy-evaluation of 1.1 mode #161

Closed

Lazy eval version w3c/json-ld-syntax#284

Merged

Lazy-Eval processingMode. w3c/json-ld-framing#76

Merged

Remove bits about setting processingMode to json-ld-1.1 automatically.

3e7afa5

gkellogg merged commit e441cc4 into master Oct 18, 2019

gkellogg deleted the lazy-eval-version branch October 18, 2019 21:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Lazy eval version #170

Lazy eval version #170

gkellogg commented Oct 16, 2019 •

edited by pr-preview bot

Loading

dlongley Oct 17, 2019

iherman Oct 17, 2019

pchampin Oct 17, 2019

gkellogg Oct 17, 2019

dlongley Oct 17, 2019

gkellogg Oct 17, 2019

pchampin left a comment

pchampin Oct 17, 2019

gkellogg Oct 17, 2019

gkellogg Oct 17, 2019

gkellogg Oct 17, 2019

gkellogg commented Oct 17, 2019

gkellogg commented Oct 17, 2019

iherman commented Oct 18, 2019

Lazy eval version #170

Lazy eval version #170

Conversation

gkellogg commented Oct 16, 2019 • edited by pr-preview bot Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pchampin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gkellogg commented Oct 17, 2019

gkellogg commented Oct 17, 2019

iherman commented Oct 18, 2019

gkellogg commented Oct 16, 2019 •

edited by pr-preview bot

Loading