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

feat: add eval test cases for empty graph and isImpliedBy #202

Merged
merged 5 commits into from
Oct 17, 2023

Conversation

jeswr
Copy link
Member

@jeswr jeswr commented Jul 31, 2023

Test cases addressing:

There may be a problem with some of these test cases as some:

  • Contain literals in the subject position
  • Contain variables

Neither of which is actually permitted in the n-quads spec. A solution for those test cases could be to have the same content for the result but contained in another n3 document. What do you think @william-vw ?

@william-vw
Copy link
Collaborator

@jeswr

A solution for those test cases could be to have the same content for the result but contained in another n3 document.

Just to be sure, you mean keeping the results in an n3 results file instead of n-triples / n-quads?

@jeswr
Copy link
Member Author

jeswr commented Aug 1, 2023

Just to be sure, you mean keeping the results in an n3 results file instead of n-triples / n-quads?

Correct

@william-vw
Copy link
Collaborator

@jeswr yes I think that makes a lot of sense!

@@ -0,0 +1,6 @@
?a <http://example.org/b1> <http://example.org/c1> _:b0 .
?a <http://example.org/b> <http://example.org/c> _:b1 .
_:b0 <http://www.w3.org/2000/10/swap/log#isImpliedBy> _:b1 _:b2 .
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jeswr yes I think that makes a lot of sense!

So it turns out that doesn't work on files like this because N3 doesn't fully support n-quads syntax, in particular having a graph term like this one.

Do you think it is worth opening up discussion as to whether this syntax should be supported in the spec @william-vw @josd .

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jeswr sorry I'm missing something - why do you want to use the n-quads syntax in these tests (as you say, it is not supported by the n3 syntax)?

Copy link
Member Author

@jeswr jeswr Aug 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally the results of eval tests are given in an n-quads format (in particular see the rdf-tests repo) as it is straightforward to parse and as such is a good syntax in which to unambiguously provide the expected result of parsing.

In terms of what I'm proposing, it is that in N3 the following:

@prefix : <http://example.org/> .
"a" :b { :c :d :e } .

Be considered equivalent to:

@prefix : <http://example.org/> .
"a" :b _:b0 .
:c :d :e _:b0 .

Parsers over in the RDF/JS world like N3.js already interpret the first statement as the following RDF/JS quads:

const result = [
quad(
  literal("a"),
  namedNode("http://example.org/b"),
  blankNode("b0"),
),
quad(
  namedNode("http://example.org/c"),
  namedNode("http://example.org/d"),
  namedNode("http://example.org/e"),
  blankNode("b0"),
)]

Copy link
Collaborator

@josd josd Aug 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In terms of what I'm proposing, it is that in N3 the following:

@prefix : <http://example.org/> .
"a" :b { :c :d :e } .

Be considered equivalent to:

@prefix : <http://example.org/> .
"a" :b _:b0 .
:c :d :e _:b0 .

They are different, the former uses a graph term, the latter uses a graph by reference.
I have added initial support for n-quads in eye and this example round trips fine:

$ cat nq2.n3
@prefix : <http://example.org/> .

"a" :b _:b0 .
:c :d :e _:b0 .
$ eye --quiet --nope nq2.n3 --pass
@prefix : <http://example.org/>.

"a" :b _:e_b0_1.
:c :d :e _:e_b0_1.

One can of course compose graph terms with the following rule in

$ cat nq3.n3
@prefix log: <http://www.w3.org/2000/10/swap/log#>.
@prefix graph: <http://www.w3.org/2000/10/swap/graph#>.
@prefix : <http://example.org/> .

"a" :b _:b0 .
:c :d :e _:b0 .
:f :g :h _:b0 .

{   ?A ?B ?C.
    ({?D ?E ?F} {?D ?E ?F ?C} ?L) log:collectAllIn ?SCOPE.
    ?G graph:list ?L.
} => {
    ?A ?B ?G.
}.
$ eye --quiet --nope nq3.n3 --pass
@prefix log: <http://www.w3.org/2000/10/swap/log#>.
@prefix graph: <http://www.w3.org/2000/10/swap/graph#>.
@prefix : <http://example.org/>.

"a" :b _:e_b0_1.
"a" :b {
    :c :d :e.
    :f :g :h.
}.
:c :d :e _:e_b0_1.
:f :g :h _:e_b0_1.
:c :d :e {
    :c :d :e.
    :f :g :h.
}.
:f :g :h {
    :c :d :e.
    :f :g :h.
}.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally the results of eval tests are given in an n-quads format (in particular see the rdf-tests repo) as it is straightforward to parse and as such is a good syntax in which to unambiguously provide the expected result of parsing.

Why would n-quads be better to provide expected results - what would the graph label indicate? I'm not finding n-quads result files in the folder you reference (aside from the rdf-n-quads folder, but those are not result files). Not trying to be difficult here, I am just wondering what the added value would be for an individual results file.

It's a much bigger discussion, for sure, but I feel that graph terms can be used to group triples belonging to the same graph (these could be made "referentially transparent" as well).

@jeswr jeswr marked this pull request as ready for review October 15, 2023 20:21
@jeswr jeswr requested a review from william-vw October 15, 2023 20:38
@jeswr
Copy link
Member Author

jeswr commented Oct 15, 2023

I've updated the files @william-vw .

I'll also respond to #202 (comment) in due course when I have time to write a full response.

Copy link
Collaborator

@william-vw william-vw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good (aside from minor comment)

@@ -1050,15 +1050,15 @@
:isImpliedBy_graphs
a test:TestN3Eval ;
mf:action <isImpliedBy/isImpliedBy_graphs.n3> ;
mf:result <isImpliedBy/isImpliedBy_graphs.nq> ;
mf:result <isImpliedBy/isImpliedBy_graphs_result.n3> ;
mf:name "isImpliedBy Eval";
rdfs:comment "<= should be parsed as log:isImpliedBy in a complex use case";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unsure what is meant by complex use case?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure either - resolved in 59f2ae7.

I don't have permissions to merge on this repo so will leave that to you.

@william-vw william-vw merged commit 73f4e00 into w3c:master Oct 17, 2023
@jeswr jeswr deleted the feat/test-cases branch October 17, 2023 14:24
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.

3 participants