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

Clarification needed for expansion of terms with @container set to @id and a local context with a @base #412

Open
niklasl opened this issue Apr 1, 2023 · 3 comments
Labels
ErratumRaised Used by the errata management spec:substantive

Comments

@niklasl
Copy link

niklasl commented Apr 1, 2023

When expanding this compact data:

{
  "@context": {
    "rdf": "http://www.w3.org/1999/02/22-rdf-syntax-ns/",
    "@vocab": "https://example.org/ns#",
    "typeDescribed": {
      "@id": "rdf:type",
      "@context": {"@base": "https://example.org/ns/"},
      "@container": "@id"
    }
  },
  "@id": "/item/1",
  "typeDescribed": {
    "Item": {"label": "Item"}
  }
}

I cannot tell from the spec (step 13.8.3.7.4 of the expansion algorithm) whether the key Item will be resolved against the local context or not. Actually, reading the spec I see nothing that sets the active context to the local context of the typeDescribed before reaching 13.8.3.7.4, although I would expect that this is the intention. Both the JSON-LD Playground and the Ruby RDF Distiller do expand the above to this:

[
  {
    "@id": "/test/item/1",
    "http://www.w3.org/1999/02/22-rdf-syntax-ns#type": [
      {
        "https://example.org/ns/label": [
          {"@value": "Item"}
        ],
        "@id": "https://example.org/ns/Item"
      }
    ]
  }
]

In the Ruby code corresponding to step 13.8.2 and onwards, IIRC, a container_context is defined and from thereon used.

Unless I've missed something in the algorithm that clarifies that something like this is required, I believe we should clarify this exhaustively in the spec, and add this test case (unless I've overlooked an existing one for this).

I've made a similar adjustment to TRLD to reflect this presumably desired behaviour.

@gkellogg
Copy link
Member

gkellogg commented Apr 1, 2023

I think it's described using the following steps:

  • Step 3 sets property-scoped context to the embedded context resolved using the active context.
  • Step 8 sets active context to that property-scoped context.
  • Step 13.8.3.4 says to "Initialize expanded index to the result of IRI expanding index.", where IRI expanding is a shortcut to expand using the active context, so that would be how "Item" is turned into "https://example.org/ns/Item".

I think the algorithm is correct, but there are definitely a lot of moving parts. If you had passed the test suite other than this newly defined test, then that would indicate that the test coverage should be improved. Please go ahead and add a PR for the new expansion test (and corresponding toRdf test).

@niklasl
Copy link
Author

niklasl commented Apr 2, 2023

I thought so too, but as my implementation is basically a transcription of the algorithm into Python (with some minor optimizations and refactored parts), I wondered why it didn't work.

Apparently, step 13.8.3.4 occurs for expansion of the index (i.e. the index key, see 13.8.3) before the active property becomes, in my example, typeDescribed. The current context is still the parent context, as is evident in step 13.5:

Initialize container mapping to key's container mapping in active context.

The key here is, with the above example, set to typeDescribed, and it is not, here, used as the active property. Thus the active context under which the index is expanded as an IRI is not the property scoped context. It is true that step 3 and 8 has been invoked by this time, but via step 13.8.3.6, within the recursive call, i.e. within its own scope. So for the index value the active context is determined from typeDescribed, and the local context becomes active. But step 13.8.3.7.4 is not invoked there, since we're processing the index value. Once the recursive call exits, the algoritm proceeds with 13.8.3.7, so when we reach 13.8.3.7.4 to expand the index, as specified, the active context is not the property scoped one.

I believe this is why your (and now also my) implementation uses a container context (which is a "sibling" to the container mapping of the key term definition in the active context). It appears to be an optimization, but it also introduces and uses this context, determined from the key, which is not spelled out to be used in the algorithm.

@gkellogg
Copy link
Member

gkellogg commented Apr 3, 2023

Yes, my analysis started at the wrong point:

You're expecting that 13.8.3.4 would set expanded index using the property-scoped context which would result in "https://example.org/ns/Item" instead of "https://example.org/ns#Item". Indeed, my code creates container context at about 13.8.2 using the following Ruby code:

# While processing index keys, if container includes @type, clear type-scoped term definitions
container_context = if container.include?('@type') && context.previous_context
  context.previous_context
elsif container.include?('@id') && context.term_definitions[key]
  id_context = context.term_definitions[key].context if context.term_definitions[key]
  if id_context.nil?
    context
  else
    log_debug("expand", depth: log_depth.to_i) {"id_context: #{id_context.inspect}"}
    context.parse(id_context, base: @options[:base], propagate: false)
  end
else
  context
end

At this point, it seems odd that I'd have such relatively complex code without something corresponding in the algorithm text. Definitely seems a substantive Erratum is in order.

@gkellogg gkellogg added spec:substantive ErratumRaised Used by the errata management labels Apr 3, 2023
@pchampin pchampin moved this to Errata in JSON-LD Management May 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ErratumRaised Used by the errata management spec:substantive
Projects
Status: Errata
Development

No branches or pull requests

2 participants