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

Issue with accessing term definitions in Expansion 11.2 #304

Closed
kasei opened this issue Jan 3, 2020 · 4 comments
Closed

Issue with accessing term definitions in Expansion 11.2 #304

kasei opened this issue Jan 3, 2020 · 4 comments

Comments

@kasei
Copy link

@kasei kasei commented Jan 3, 2020

Expansion step 11.2 says:

For each term which is a value of value ordered lexicographically, if term is a string, and term's term definition in active context has a local context, set active context to the result Context Processing algorithm, passing active context, the value of the term's local context as local context, and false for propagate.

I think this text is ambiguous about the order that processing must occur. Based on test tc018, I believe accessing term definitions from active context must occur outside of the loop body where active context is updated.

If 11.2 is interpreted (incorrectly) as follows, tc018 will fail:

for each term in sorted(values of value):
	if term is a string:
		tdef = term_definition(in: active context, for: term)
		if '@context' in tdef:
			active_context = context_processing(active context, tdef['@context'], propagate: false)

This will clear active context on the first pass of the loop, and there will be no relevant term definition available in the second pass.

@gkellogg

This comment has been minimized.

Copy link
Member

@gkellogg gkellogg commented Jan 4, 2020

I think what's supposed to happen is to check term's term definition in type-scoped context to see if it has a local context, and set active context to the result of processing it from the active context, so we don't loose any redifinitions along the way.

More like the following:

for each term in sorted(values of value):
    if term is a string:
        tdef = term_definition(in: **type-scoped context**, for: term)
	if '@context' in tdef:
		active_context = context_processing(active context, tdef['@context'], propagate: false)
@gkellogg

This comment has been minimized.

Copy link
Member

@gkellogg gkellogg commented Jan 4, 2020

@davidlehn, WDYT?

gkellogg added a commit that referenced this issue Jan 8, 2020
…ot _active context_, which is updated along the way.

For issue #304.
@gkellogg

This comment has been minimized.

Copy link
Member

@gkellogg gkellogg commented Jan 8, 2020

@kasei I used that interpretation described above in PR #307. Please indicate if this adequately addresses the issue.

@gkellogg gkellogg added this to Editorial work complete in JSON-LD Management Jan 8, 2020
@kasei

This comment has been minimized.

Copy link
Author

@kasei kasei commented Jan 13, 2020

@gkellogg using type-scoped context does seem to resolve the issue. If that's the intent, then this change looks good. Thanks.

gkellogg added a commit that referenced this issue Jan 13, 2020
…ot _active context_, which is updated along the way.

For issue #304.
@gkellogg gkellogg closed this Jan 13, 2020
@gkellogg gkellogg removed this from Editorial work complete in JSON-LD Management Jan 13, 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.