Skip to content

tests related to importing of blank nodes#4

Merged
hartig merged 7 commits intomainfrom
bnode-tests
May 29, 2024
Merged

tests related to importing of blank nodes#4
hartig merged 7 commits intomainfrom
bnode-tests

Conversation

@hartig
Copy link
Contributor

@hartig hartig commented May 19, 2024

This PR adds several tests related to the spec text that we are still working on in PR #2. That is, these tests are about blank node identifies, reading them from one or multiple Turtle files, from SPARQL queries, or from both Turtle files and SPARQL queries.

@kasei please review. I think most of them are quite straightforward. The only one that is probably controversial is :bnodes-sparql-10 which uses the following query.

ASK {
	BIND( "[_:b, 42]"^^cdt:List AS ?list1 )
	BIND( "[_:b, 43]" AS ?str )
	BIND( STRDT(?str, cdt:List) AS ?list2 )

	BIND( cdt:get(?list1,1) AS ?e1 )
	BIND( cdt:get(?list2,1) AS ?e2 )

	FILTER( isBLANK(?e1) )
	FILTER( isBLANK(?e2) )
	FILTER( ?e1 = ?e2 )
}

I am unsure myself about this one. It just came to my mind that something like this is possible and there may be expectations for this to work. Yet, strictly speaking, this test should not pass; not according to the current spec text that we have in PR #2 because that spec text focuses only on cdt:List and cdt:Map literals that are given directly with their lexical forms. So, perhaps, the correct version of this test should say FILTER( ?e1 != ?e2 ) instead of FILTER( ?e1 = ?e2 ) !?

@hartig hartig requested a review from kasei May 19, 2024 09:32
@hartig
Copy link
Contributor Author

hartig commented May 26, 2024

@kasei I added more tests (see commit 0c47b81). These new tests cover cases in which the CDT literals are nested such as the following two examples.

ex:s ex:p "[_:b, 42, [_:b] ]"^^cdt:List .
ex:s ex:p "[_:b, 42, '[_:b]'^^<http://w3id.org/awslabs/neptune/SPARQL-CDTs/List> ]"^^cdt:List .

hartig added a commit to hartig/jena that referenced this pull request May 26, 2024
…tifiers extends to all CDT literals within a file as well as to the file itself; as per awslabs/SPARQL-CDTs#2 and awslabs/SPARQL-CDTs#4
@kasei
Copy link
Collaborator

kasei commented May 26, 2024

I will review them soon.

The only one that is probably controversial is :bnodes-sparql-10 which uses the following query.

Interesting test. I'm not sure STRDT (or casting/constructor functions) should do bnode rewriting. Maybe there's a reason to think it shouldn't allow any lexical form with a bnode in it? Otherwise you let in the accidentail skolemization again. I'm sure I'll have more to say after going through the tests.

For what it's worth, my implementation currently fails 9 out of the 73 tests.

@hartig
Copy link
Contributor Author

hartig commented May 27, 2024

I have worked on my implementation the last two days and it passes all these tests now, except for the STRDT test.

I didn't necessarily mean that we need to include this test. It is just something that came to my mind and I wanted to bring it up. We may also include the inverse of this test.

hartig added a commit to hartig/jena that referenced this pull request May 27, 2024
…tifiers extends to all CDT literals within a file as well as to the file itself; as per awslabs/SPARQL-CDTs#2 and awslabs/SPARQL-CDTs#4
@kasei
Copy link
Collaborator

kasei commented May 28, 2024

OK. I've now gone over all of the tests. My code now also passes all tests except bnodes-sparql-10, which I think we need to discuss more.

@hartig
Copy link
Contributor Author

hartig commented May 29, 2024

Thanks for checking the tests! I have removed bnodes-sparql-10 for the moment, to merge this PR. After that I will create a separate PR with bnodes-sparql-10.

@hartig hartig merged commit 2057fca into main May 29, 2024
@hartig hartig deleted the bnode-tests branch May 29, 2024 07:30
hartig added a commit to hartig/jena that referenced this pull request Jun 4, 2024
…tifiers extends to all CDT literals within a file as well as to the file itself; as per awslabs/SPARQL-CDTs#2 and awslabs/SPARQL-CDTs#4
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.

2 participants