Corrected sparqlquery for removing triples#351
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #351 +/- ##
==========================================
+ Coverage 77.49% 77.77% +0.27%
==========================================
Files 22 22
Lines 2515 2515
==========================================
+ Hits 1949 1956 +7
+ Misses 566 559 -7 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Would it make sense to merge this file with sparqlwrapper.py?
If not, we should update the module docstring, since it now tests both graphdb and fuseki.
The graphdb_available() and fuseki_available() help functions can be merged to a single function taking the check URL as argument.
Possible to take these suggestions in a separate PR.
There was a problem hiding this comment.
I do not think it makes sense to merge it with the test_sparqlwrapper.py as that is justy about trying out one sparql query on wikipedia. I renamed that test.
graphdb_available and fuseki_available have been merged to triplestore_available.
docstring updated.
| for name, value in zip("spo", triple) | ||
| ) | ||
| query = f"DELETE {{ {spec} }}" | ||
| query = f""" |
There was a problem hiding this comment.
Very nice. Yes, this has clearly not been tested before.
We should also add support for general DELETE queries in the query() method.
There was a problem hiding this comment.
No, this is not in agreement with the current documentation of query() in the base triplestore.py. It says:
Note:
This method is intended for SELECT, ASK, CONSTRUCT and
DESCRIBE queries. Use the update() method for INSERT and
DELETE queries.
Not all backends may support all types of queries.
Can be added as an issue, but I am not in favour of adding issues that we do not know if we want to prioritize. The issue list becomes too long...
jesper-friis
left a comment
jesper-friis
left a comment
There was a problem hiding this comment.
I am happily approving this. But please take a look at the comments. Do you want to include them here or should we put them in a new issue?
jesper-friis
left a comment
jesper-friis
left a comment
There was a problem hiding this comment.
Good work. Approved
2 participants
Description
Removing triples in the sparqlwrapper was not done correctly.
Type of change
Checklist for the reviewer
This checklist should be used as a help for the reviewer.