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

Undefined variable in Node Map Generation #276

Closed
kasei opened this issue Dec 23, 2019 · 9 comments
Closed

Undefined variable in Node Map Generation #276

kasei opened this issue Dec 23, 2019 · 9 comments

Comments

@kasei
Copy link

@kasei kasei commented Dec 23, 2019

Node Map Generation step 3 says:

If element has an @type entry, perform for each item the following steps:

However, item here does not seem to be defined.

@gkellogg

This comment has been minimized.

Copy link
Member

@gkellogg gkellogg commented Dec 23, 2019

This text defines item, as in "for each item in @type". It seems to me to be clear that we're iterating over values of @type.

@gkellogg gkellogg added the wr:open label Dec 23, 2019
@gkellogg gkellogg added this to Discuss-Call in JSON-LD Management Dec 23, 2019
@kasei

This comment has been minimized.

Copy link
Author

@kasei kasei commented Dec 23, 2019

@gkellogg there are many uses of "for each" in the spec (50+), but I believe this is the only one where there is not explicit language regarding what list/map data is being iterated over.

@gkellogg

This comment has been minimized.

Copy link
Member

@gkellogg gkellogg commented Dec 24, 2019

Text updated in #285.

@kasei please acknowledge.

@kasei

This comment has been minimized.

Copy link
Author

@kasei kasei commented Dec 24, 2019

@gkellogg, looks good. Thanks!

@kasei

This comment has been minimized.

Copy link
Author

@kasei kasei commented Dec 24, 2019

Followup question: given the text of step 3 in #285:

For each item in the @type entry of element, if any:

Must the @type entry be an array? For example, what happens when handling data such as the output from expand test t0002 when processing the value of the "http://example.com/term2" key?

[{
  "@id": "http://example.com/id1",
  "@type": ["http://example.com/t1"],
  "http://example.com/term1": [{"@value": "v1"}],
  "http://example.com/term2": [{"@value": "v2", "@type": "http://example.com/t2"}],
  "http://example.com/term3": [{"@value": "v3", "@language": "en"}],
  "http://example.com/term4": [{"@value": 4}],
  "http://example.com/term5": [{"@value": 50}, {"@value": 51}]
}]
@gkellogg

This comment has been minimized.

Copy link
Member

@gkellogg gkellogg commented Dec 24, 2019

After expansion, @type is an array for node objects, but not for value objects. The exception is when framing, when value objects will also use an array for @type.

Certainly, when processing a value object, you would be presented with "http://example.com/t2" and not ["http://example.com/t2"], so it depends on if it is ambiguous what each item in the @type entry means if it's not an array. Rather than adding more wording about coercing it to an array, I would rather keep the text simpler, assuming that the meaning is clear. Of course, depending on your programming environment, you may need to ensure that you can iterate over these values. In my case, I implicitly coerce such values to an array before iterating.

Do you think that needs to be spelled out?

@kasei

This comment has been minimized.

Copy link
Author

@kasei kasei commented Dec 26, 2019

@gkellogg Given that step 3 is not just about iterating over elements, but about iterating and possibly replacing those values, yes, I do think the text needs to be explicit. And given the replacement, I'm not sure the usual text about coercing into an array if necessary (used elsewhere in the document) will work here. That is, assuming I'm understanding the intent to leave the type of the element alone and replace either the value (if scalar) or some of its elements (if array).

In general, I think I'd always prefer these cases be explicit about any required coercion, because languages and type systems differ in their capabilities (implicit coercions, ability to treat varied types as collections, etc.). Better to be be explicit than to make the assumption that the meaning is clear to both the reader and their language/environment.

@gkellogg

This comment has been minimized.

Copy link
Member

@gkellogg gkellogg commented Dec 27, 2019

I changed 3 to be more explicit, but I generally disagree about so explicit, where I feel the algorithmic intent is clear.

Again, please acknowledge.

@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 yes, the updated text sufficiently addresses my concern. 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.