w3c / json-ld-api

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

Handling `@context` in Create Term Definition #621

Open multimeric opened 3 days ago

multimeric commented 3 days ago

I have some concerns about section 21 of 4.2 Create Term Definition algorithm:

It doesn't make sense to me for a few reasons:

  1. In 21.4 we set the "local context" of a term definition. However a term definition only has a property called "context" and not "local context", as defined in section 4.1 Context Processing Algorithm
  2. The context variable which we pull from the @context key is, at this stage, just a map. It doesn't become a context object until we execute the context processing algorithm on it. However, we throw away the result of this algorithm in 21.3 and instead just set the term definition's context to context in 21.4, which is the "raw" unprocessed version. The same section as above specifies that the type of context for a Term Definition is context, which to me means a processed context, the result of the context processing algorithm.

Therefore I think the fix is to remove the "note" from this section, and instead assign term_definition.context = create_term_definition(context, ...). Happy to put in a PR.

gkellogg commented 1 day ago

I have some concerns about section 21 of 4.2 Create Term Definition algorithm:

BTW, you can see the rendered editor's draft on GitHub here.

It doesn't make sense to me for a few reasons:

  1. A term definition doesn't have a property called "local context", it just has a property called "context", as defined in section 4.1 Context Processing Algorithm

I believe you're correct, and that it is the context of definition which should be set to context.

  1. The context variable which we pull from the @context variable is, at this stage, just a map. It doesn't become a context object until we execute the context processing algorithm on it. However, we throw away the result of this algorithm and instead just set the term definition's context to context, which is the "raw" unprocessed version. The same section as above specifies that the type of context for a Term Definition is context, which to me means a processed context, the result of the context processing algorithm.

The purpose of calling the Context Processing algorithm on the extracted context is to validate it; that is why the un-processed version (array, string, or map) is set in the term definition and not the processed context. It can't be fully processed until it's used in the proper scope (that's why it's a 'scoped context". Then it's processed , it will be combined on top of any existing context state, which could be different each time it's used.

Therefore I think the fix is to remove the "note" from this section, and instead assign term_definition.context = create_term_definition(context, ...). Happy to put in a PR, I just can't find the part of the repo where the latest spec is actually built from.

So, this would not be valid given the way the algorithms are intended to use scoped contexts.

multimeric commented 1 day ago

Thanks, I take your point about the scoping which I admit I don't fully understand. Can you comment on the issue of type checking though? How can we assign a context to be a JSON map when it's defined in the spec as a context object? This actually breaks my code because I defined separate types for e.g.active_context: Context (a custom class) and local_context: Dict[str, Json] (where Json is any JSON object). So when we assign definition.context = context, the type checker rightly flags this as a type error.

gkellogg commented 1 day ago

The spec says that a term definition consists of several entries including an optional context. Context is defined as a set of rules for interpreting a JSON-LD document, which I think is consistent with the unprocessed value of @context. A context definition would be the processed form of a context.

multimeric commented 1 day ago

Thanks, in that case I will interpret the spec as meaning that "a term definition has a field called context which is a local, unprocessed JSON map", or in Python code:

class TermDefinition:
    context: dict[str, JSON]
    # Other fields omitted

My confusion is part of a greater issue about the ambiguity of the data type of context objects in the spec. In general when it says "local context" in means an unprocessed map, and when it says "active context" in means a processed context (what you call a context definition). However this is not consistent, as demonstrated by a term definition having a field called context which is an unprocessed map. I think this could be improved by:

I suppose this could be considered a separate issue to my original concern, so I'm happy to file it separately.