w3c / json-ld-api

JSON-LD 1.1 Processing Algorithms and API Specification
https://w3c.github.io/json-ld-api/
Other
76 stars 29 forks source link

Handling active subject in node map generation #519

Closed clehner closed 3 years ago

clehner commented 3 years ago

Node Map Generation step 2 says "If the active subject is null, set node to null otherwise reference the active subject entry of graph using the variable subject node." This seems to imply that if active subject is not null then it must be a string so as to be a map key. But active subject may also be a map, as when called from step 6.9.3.1.1 and handled in step 6.5. In my implementation, if active subject at step 2 is a map or anything other than a string, I treat that as if it was null. Is that correct? Or should we take the @id entry of the map instead?

I am also unsure about how subject node is supposed to be handled when not set. It is referenced later in the algorithm without any checking, for example: 4.1.1) If subject node does not have an active property entry, create one and initialize its value to an array containing element. What is supposed to happen here if subject node has not been set? In my implementation I throw an error.

Step 2 says to set node to null if the active subject is null. But I don't see node referenced later in the algorithm before re-setting it at step 6.4. Should step 2 say to set subject node to null instead? That would remove the apparently used variable, and also make sure subject node is initialized. But this would still not fully explain what to do if subject node is null when it is later referenced in e.g. step 4.1.1.

pchampin commented 3 years ago

I will answer your questions in reverse order.

I'll make a PR to clarify these points.

pchampin commented 3 years ago

@iherman since the changes in #520 are only editorial, does this issue still needs to be flagged as an erratum?

iherman commented 3 years ago

@iherman since the changes in #520 are only editorial, does this issue still needs to be flagged as an erratum?

Let us be conservative: yes. Keep the 'Editorial' flag that I have just added, and that will be displayed in the report accordingly.

clehner commented 3 years ago

@pchampin Thanks for clarifying and addressing it.

gkellogg commented 3 years ago

PR is merged. As this is really just editorial, I don't think it really reservers an erratum tag, but that's up to @iherman. We can close this issue if @clehner is satisfied.

clehner commented 3 years ago

Fine by me. Thanks all.

iherman commented 3 years ago

I think it is better to keep it as an erratum. After all, an erratum is relative to the official publication that hasn't changed.

gkellogg commented 3 years ago

Summary: Improved the description of the Node Map Generation Algorithm to handle the case where active subject is a map, which is treated as if it were null. Also, notes areas in the algorithm where the node subject will always be a map.

gkellogg commented 3 years ago

@iherman change from ErratumRaised to Erratum when you're satisfied.

iherman commented 3 years ago

@iherman change from ErratumRaised to Erratum when you're satisfied.

Done. The issue can be closed, too, the errata page should show it regardless.