Add support for empty graphs in the subject and object positions#374
Add support for empty graphs in the subject and object positions#374Dexagod wants to merge 2 commits intordfjs:versions/2.0.0from
Conversation
|
@Dexagod Are empty graphs supported by both N3 (seems yes) and RDF-star (citation needed)? |
RubenVerborgh
left a comment
RubenVerborgh
left a comment
There was a problem hiding this comment.
It's definitely not true in subject position for RDF-star, so we will need a format check.
|
(I might be overcautious here, and maybe this code doesn't hit with RDF-star; just want to be sure.) |
| const outerGraph = this._contextStack[this._contextStack.length - 1].graph; | ||
| this._emit( | ||
| graph, | ||
| this._namedNode('http://www.w3.org/1999/02/22-rdf-syntax-ns#value'), |
There was a problem hiding this comment.
Probably want to use a constant for this.
| this._emit( | ||
| graph, | ||
| this._namedNode('http://www.w3.org/1999/02/22-rdf-syntax-ns#value'), | ||
| this._literal('true', this._namedNode('http://www.w3.org/2001/XMLSchema#boolean')), |
There was a problem hiding this comment.
Probably want to use a constant for this.
There was a problem hiding this comment.
This was an error on my part.
I represented the empty graph as a blank node with an rdf:value of "true"^^xsd:boolean,
whereas I was corrected that it should be directly replaced with "true"^^xsd:boolean.
| if (this._subject !== null) { | ||
| // Catch the empty graph being closed when parsing N3. | ||
| // In this case, we emit the empty graph as the value "true"^^xsd:boolean | ||
| if (!this._predicate && !this._object) { |
There was a problem hiding this comment.
Is it sufficient to check that there is no predicate?
There was a problem hiding this comment.
The predicate check should be sufficient, as the flow goes as follows:
at the start of the formula the this._readSubject function is called. This one sets the this._predicate value to null.
As the token here is }, the this._readPunctuation function is called, that then calls the this._readFormulaTail function.
In here, we find this code. So I concluded that we should only check on the this._predicate value being null, and not the this._object, as the predicate value alone should be enough of an indicator and I am unsure how the object gets there in all flows.
There was a problem hiding this comment.
Okay. so let's do if (this._predicate === null) indeed.
|
@Dexagod indeed we will only want to do this in N3 mode and error otherwise. @RubenVerborgh this PR is not about RDF-Star; it is specifically for N3 content types in order to align with the decisions made in w3c/N3#185 & rdfjs/types#37 in the next major release. In particular note that there are now N3 spec tests for what has been implemented here w3c/N3#202. |
|
Ack about it being for N3; just want to make sure it doesn't fire at the wrong time. But perhaps |
|
Yep, synced on it with Jos and I indeed was mistaken. I'm setting it straight atm in terms of the true needing to be injected directly instead of with rdf:value. I'll double check to make sure the routine only fires in N3 mode. |
|
Empty graphs are now correctly represented as "true"^^xsd:boolean. I checked and the Should I check if RDF-star is enabled and throw an error in case it is? |
No, I do not think we should error when N3 and star are both enabled. There is just no particular behavior we have to comply with as there is no N3-star spec. |
3 participants
This PR contains a fix for parsing empty graphs in subject and object positions when parsing empty graphs.
I added some test cases that check for empty graphs in the subject and object position, both in the default graph, and contained in a subgraph.
Note that this does NOT work for empty graphs in the predicate position.
I have added tests for these as well, but they are commented out at the moment.