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

Problem testing non-expanded value in Expansion algorithm 13.4.7.1 #269

Closed
kasei opened this issue Dec 20, 2019 · 7 comments
Closed

Problem testing non-expanded value in Expansion algorithm 13.4.7.1 #269

kasei opened this issue Dec 20, 2019 · 7 comments

Comments

@kasei
Copy link

@kasei kasei commented Dec 20, 2019

Expansion Algorithm step 13.4.7.1 says:

If input type is @json, set expanded value to value. If processing mode is json-ld-1.0, an invalid value object value error has been detected and processing is aborted.

I believe the initial test in this step should be against the expansion of input type. Test tjs16 demonstrates this by using a key json that maps to @json.

@gkellogg

This comment has been minimized.

Copy link
Member

@gkellogg gkellogg commented Dec 23, 2019

@kasei, text is updated in PR #275 to use the IRI Expansion algorithm to get the result for comparison. Please indicate if this solves the issue.

@kasei

This comment has been minimized.

Copy link
Author

@kasei kasei commented Dec 24, 2019

@gkellogg I do not understand the updated text in #275. Ignoring the trailing error handling text, it now reads:

If input type is not null, it was determined in step 12 that element is a value object representing a JSON literal. Set expanded value to value.

Is "it was determined in step 12 that element is a value object representing a JSON literal" meant to be a note, or a conjunctive clause? If the former, I think it needs note styling. If the latter, I'm confused by both the trailing period (should it be a comma?) and the removal of the explicit mention of the @json test (which is now neither mentioned in 13.4.7.1 or in 12).

@gkellogg

This comment has been minimized.

Copy link
Member

@gkellogg gkellogg commented Dec 24, 2019

It is descriptive, and if removed, would not change the meaning. It could be a note, or an inline clause. The trailing period should probably, instead, but a semi-colon.

The removal of mentioning @json, is because, as indicated, in step 12, input type will only be set to a non-null value if it is an @type who's value expands to @json, so a further test is redundant. We could have changed step 12 to not predicate setting input type on it's being @json, but then would potentially need to record both an expanded input type and input type (the original unexpanded value), and it's not used anywhere else in the algorithm. Rather than re-write that, it seemed simpler to just look to see if input type is not null.

We could consider renaming input type to something like type is json.

@kasei

This comment has been minimized.

Copy link
Author

@kasei kasei commented Dec 24, 2019

@gkellogg, It's not obvious to me based on the text of 12 why "input type will only be set to a non-null value if it is an @type who's value expands to @JSON." I don't see any reference to JSON in step 12. Can you help me understand this? Is this just something that falls out of the spec as a whole that isn't explicit in the text of step 12?

@gkellogg

This comment has been minimized.

Copy link
Member

@gkellogg gkellogg commented Dec 24, 2019

No, I was mis-reading it before, for some reason. I think we can change the language for establishing input type to set it to the expanded value. Note that we avoid a bunch of discussion on how to actually expand either the entry or its value. We could probably solve this by combining them together as follow:

Initialize input type to the expansion of the last value of the first entry in element expanding to @type (if any), ordering entries lexicographically by key. Both entry key and value are expanded using the IRI Expansion algorithm, passing active context and true for vocab.

Step 13.4.7.1 then reverts to checking if input type is @json.

Step 13.4.16 would then be "Unless expanded value is null, and expanded property is @value, and input type is not @json, set the expanded property entry of _result_to expanded value."

This would through way null values of @value, unless input type is @json, which allows null to be used as a JSON literal.

@gkellogg

This comment has been minimized.

Copy link
Member

@gkellogg gkellogg commented Dec 24, 2019

Updated PR #275 to use that language, brining it closer to the original.

@gkellogg gkellogg moved this from Discuss-Call to Editorial work complete in JSON-LD Management Dec 27, 2019
@kasei

This comment has been minimized.

Copy link
Author

@kasei kasei commented Dec 30, 2019

@gkellogg changes look good, thanks!

@gkellogg gkellogg closed this Jan 10, 2020
@gkellogg gkellogg removed this from Editorial work complete in JSON-LD Management Jan 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.