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

toRdf tests for @prefix #77

Closed
wants to merge 4 commits into from
Closed

Conversation

rubensworks
Copy link
Member

This PR does two things:

  1. It modifies toRdf-0088 so that the compact IRI term definition that ends with a non-gen-delim character is removed, as this is not allowed anymore unless @prefix is used. This is needed, as this is a breaking change in 1.0:

    This represents a small change to the 1.0 algorithm to prevent IRIs that are not really intended to be used as prefixes from being used for creating compact IRIs.

  2. toRdf-0130 is added as an extension of toRdf-0088, and makes use of this new @prefix feature.

@rubensworks rubensworks changed the title Feature/prefix toRdf tests for @prefix Apr 8, 2019
@rubensworks
Copy link
Member Author

I did not add any tests for compact IRIs with term definition not ending with a gen-delim for two reasons:

  1. I am not sure if these cases should result in an error or ignoring the term (I assume the latter).
  2. If an error should be thrown, no NegativeEvaluationTests exist in the toRdf suite, so I suspect there may be a reason for this (is there?).

@gkellogg
Copy link
Member

gkellogg commented Apr 8, 2019

Tests are failing because some files from json-ld-wg/common need to be copied over.

I’ll look at the changes in more detail when I get home in a day or so.

It is intended that Compact IRIs using a term not ending in a gen-delim are treated as if there were no term definition, so it will likely be treated as an absolute IRI. Generating an error might be heavy handed, a warning would be useful. We could consider ignoring the Compact IRI as well, like an undefined term.

@rubensworks
Copy link
Member Author

It is intended that Compact IRIs using a term not ending in a gen-delim are treated as if there were no term definition, so it will likely be treated as an absolute IRI. Generating an error might be heavy handed, a warning would be useful. We could consider ignoring the Compact IRI as well, like an undefined term.

Updated the tests to reflect this.

@gkellogg
Copy link
Member

We'll need some spec-text to ignore such Compact IRIs. IRI Expansion step 5.4 will need some discussion to qualify the term for use, considering gen-delim and @prefix definitions. But, this might not be an acceptable change considering backwards compatibility. So, we may not be able to pull this PR.

@gkellogg gkellogg added this to Discuss-GH in JSON-LD Management Apr 13, 2019
@azaroth42 azaroth42 removed this from Discuss-GH in JSON-LD Management Apr 19, 2019
@BigBlueHat BigBlueHat mentioned this pull request May 3, 2019
@iherman
Copy link
Member

iherman commented May 4, 2019

This issue was discussed in a meeting.

  • RESOLVED: dlongley and pchampin to craft alternate proposal to #76 with @prefix having 3 values
View the transcript More compact @Prefix
Benjamin Young: link: #76
Gregg Kellogg: related: #77
Benjamin Young: proposal from rubenworks, to make it simpler to declare @Prefix terms
Ruben Taelman: a similar thing is already possible with @reverse, so it should not be too hard to add in implementations
Dave Longley: is this an error?
… what do we do when @id and @Prefix are both strings and they disagree? is that an error?
… my understanding is that the proposal is that "@Prefix": `` is just syntactic magic for `"@id": ``, "@Prefix": true` … not a replacement
Gregg Kellogg: it is with @reverse, so it should be an error, yes
… there is a point in the issue about terms which are not appropriate as prefixes
… there are two aspects in the PR: 1/ use @Prefix where the value is an IRI, while the current practice is to use @id with an IRI, and @Prefix is a boolean. Should we obsolete the current practice?
… 2/ what if the value of @Prefix is a pname?
Ruben Taelman: [answers to gkellogg]
Gregg Kellogg: we must keep in mind the JSON 1.0 behavior
Ruben Taelman: I see this as an alternate representation
Ivan Herman: does the combination of @id and @Prefix:false come up in practice?
Gregg Kellogg: there are reasons to use it. I don’t know if it is used in the wild?
Ivan Herman: that would be a reason to keep the current practice, and consider the new version as a shorthand
… and I wouldn’t be shocked if the new notation @Prefix:IRI was roundtripped to @id+@Prefix:true
Gregg Kellogg: we don’t roundtrip contexts anyway
Benjamin Young: there seems to be a consensus on considering the proposal as a shorthand for the current syntax
Benjamin Young: https://w3c.github.io/json-ld-syntax/#compact-iris (for current text)
Gregg Kellogg: there are some implications; must be some working on the implications of combinations of @id and @Prefix
… you probably would not use the prefix term as a property
Dave Longley: perhaps if @Prefix is a string, then also including @id is an error … and then process @Prefix as a string first and do the transformation to @id: `, @Prefix: true`
Benjamin Young: https://w3c.github.io/json-ld-syntax/#example-32-using-explicit-prefix-declaration-to-create-compact-iris
Benjamin Young: @id+@Prefix:true feels a lot like using @vocab
Gregg Kellogg: it works a little differently in the compactIRI algo
… we are going contrary the trend of moving compact IRIs away
… the reason it is there is that, in 1.0, many terms were used to build compact IRIs, but were not intended to do that
… eg: Sport := schema:Sport, then using Sport:Event
@Prefix was a way of capturing the intention of the creator of the term
Dave Longley: lots of unwanted CURIEs being greedily generated in 1.0
Benjamin Young: strange compact IRIs can be used maliciously, to misdirect users to an unexpected endpoint URL
Benjamin Young: w3c/json-ld-syntax#155
Dave Longley: I like the syntax, makes it more obvious that the term is intended to be used as a prefix
… even though I don’t like CURIEs in general
… in JSON-LD
… It was a mistake in 1.0 to allow any term to be used as a prefix
Benjamin Young: curie spec https://www.w3.org/TR/curie/
Benjamin Young: I agree with that
Pierre-Antoine Champin: Proposal is not to make @Prefix:false the default?
Dave Longley: No
Gregg Kellogg: we did change 1.0 behavior by limiting the terms that can be used as prefixes
… only IRIs ending with gen-delims can be used as prefixes
Dave Longley: instead of just “property”
Benjamin Young: gendelim https://tools.ietf.org/html/rfc3986#section-2.2
Ivan Herman: what does @Prefix:false means?
Ivan Herman: for the records: https://tinyurl.com/yxw7qv98 (example)
Gregg Kellogg: it means that we do not want this term to be used by compaction to generate a compact IRI
Gregg Kellogg: https://w3c.github.io/json-ld-syntax/#example-32-using-explicit-prefix-declaration-to-create-compact-iris
Gregg Kellogg: for expansion, it does not matter whether @Prefix is true or false
Dave Longley: if you click on compact in the link above
… by copying the context to the right and changing the value of @Prefix
… you can see the difference
… but you can’t make a term only a prefix, this is a 1.0 problem, we can’t go back
Gregg Kellogg: but we could define @Prefix:IRI to mean exactly that
… but we still mean @Prefix:false
Dave Longley: yes, but that would differ from what we said earlier. This is not syntactic sugar anymore
Pierre-Antoine Champin: another way would be add 3 values for @Prefix
Rob Sanderson: +1 to PA
Pierre-Antoine Champin: @default would say you can use this prefix as a term
… false as you can not use this as prefix
… true, the proposed new meaning that gregg proposed about @Prefix IRI
… this way we can keep backwards compatibility with 1.0
Proposed resolution: dlongley and pchampin to craft alternate proposal to #76 with @Prefix having 3 values (Benjamin Young)
Gregg Kellogg: +1
Adam Soroka: +1
Benjamin Young: +1
Ruben Taelman: +1
Ivan Herman: +1
Dave Longley: +1 sure
David I. Lehn: +1
Dave Longley: the third value would be an IRI, and we’re back to what gkellogg just proposed
Rob Sanderson: +1
Resolution #2: dlongley and pchampin to craft alternate proposal to #76 with @Prefix having 3 values

@gkellogg
Copy link
Member

We've deferred this feature of @prefix to a future version.

@gkellogg gkellogg closed this Jul 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants