Skip to content

missing instructions in Node Map generation #549

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

Open
pchampin opened this issue Oct 8, 2022 · 3 comments
Open

missing instructions in Node Map generation #549

pchampin opened this issue Oct 8, 2022 · 3 comments

Comments

@pchampin
Copy link
Contributor

pchampin commented Oct 8, 2022

I believe that an instruction is missing from step 6.10 of Node Map Generation, namely:

  • if node map does not contain an id entry, create one and initialize its value to an empty map

before recursively invoking the algorithm.
Indeed, in step 2 of the invoked algorithm, that entry is assumed to already exist:

Reference the map which is the value of the active graph entry of node map")

(edited wrong markup, which made the suggested sentence potentially harder to understand)

@gkellogg
Copy link
Member

gkellogg commented Oct 8, 2022

Sorry, a bit confused. Step 6.10 is the following:

6.10) If element has an @graph entry, recursively invoke this algorithm passing the value of the @graph entry for element, node map, and id for active graph before removing the @graph entry from element.

In fact, when searching the document (both TR and ED), I can't find a sentence "if node map does not contain an id entry, create one and initialize its value to an empty map".

The algorithm takes node map as an input argument, so it definitely must already exist. The places which recursively invoke the algorithm pass the existing node map, so I don't see the specific issue.

@pchampin
Copy link
Contributor Author

The sentence I wrote ("if node map does not contain an id entry...") is what I suggest to add to step 6.10 . So indeed, it is currently not in the spec :-)

Of course node map always exists. What does not always exist, when entering 6.10, is an entry in node map whose key is id. When the algorithm is recursively invoked, passing node map and passing id for active graph, this entry is assumed to exist at step 2.

By adding the suggested sentence in 6.10 before the recursive invocation, we ensure that this assumption in the invoked step 2 is satisfied.

@pchampin pchampin moved this to Errata in JSON-LD Management May 15, 2024
@gkellogg gkellogg added the class-3 Class-3 change label Dec 3, 2024
@niklasl
Copy link
Contributor

niklasl commented May 4, 2025

I agree that what @pchampin suggests seems to be needed. (I've made a similar note in my implementation.)

I've later on also noticed that this node map generation algorithm mutates the input. Several steps (6.7, 6.9, 6.10, 6.11) says to remove an entry from the element. Therefore, I think step 2, where element is a map, also needs to make a (shallow) copy of this element, which is then subsequently mutated. (In 6.12, the remaining keys of the element are used, so there are other ways of fixing this; making a copy is just the easiest change to the current design.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Errata
Development

No branches or pull requests

3 participants