Added update() method to sparqlwrapper backend by jesper-friis · Pull Request #356 · EMMC-ASBL/tripper

jesper-friis · 2025-03-11T13:03:09Z

Description

Added update() method to sparqlwrapper backend supporting both INSERT and DELETE queries.

Other fixes:

fixed failing test by empty the triplestore as the first thing in the test
made it easier to run the populate_and_search() test in test_sqarqlwrapper_graphdb_fuseki.py without pytest.

Type of change

Bug fix and code cleanup
New feature
Documentation update
Testing

Checklist for the reviewer

This checklist should be used as a help for the reviewer.

Is the change limited to one issue?
Does this PR close the issue?
Is the code easy to read and understand?
Do all new feature have an accompanying new test?
Has the documentation been updated as necessary?
Is the code properly tested?

…te-query

codecov · 2025-03-11T13:10:21Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.91%. Comparing base (004f05e) to head (0de7c6b).
Report is 11 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #356      +/-   ##
==========================================
+ Coverage   77.77%   77.91%   +0.14%     
==========================================
  Files          22       22              
  Lines        2515     2522       +7     
==========================================
+ Hits         1956     1965       +9     
+ Misses        559      557       -2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

francescalb · 2025-03-11T13:13:45Z

tests/backends/test_sparqlwrapper_graphdb_fuseki.py

+def get_triplestore(tsname: str) -> "Triplestore":
+    """Help function that returns a new triplestore object."""
+    from tripper import Triplestore
+
+    if tsname == "GraphDB":
+        ts = Triplestore(
+            backend="sparqlwrapper",
+            base_iri="http://localhost:7200/repositories/test_repo",
+            update_iri=(
+                "http://localhost:7200/repositories/test_repo/statements"
+            ),
+        )
+    elif tsname == "Fuseki":
+        ts = Triplestore(
+            backend="sparqlwrapper",
+            base_iri=f"{FUSEKI_CHECK_URL}/test_repo",
+            update_iri=f"{FUSEKI_CHECK_URL}/test_repo/update",
+            username="admin",
+            password="admin0",
+        )
+    else:
+        raise ValueError(f"Unsupported triplestore name: {tsname}")
+
+    return ts


I disagree with moving this out of the actual test for each triplestore. There is reduction in duplication of code here, and the test as it is, is so small already that moving things out makes it less readable.

The point with this is to make it easy to run the test without pytest for debugging.

Now I can do:

if True: tsname = "Fuseki" #def populate_and_search(tsname): ...

You could run if True on test_fuseki and test_graphdb directly before.

That will not allow me to debug. I want to be able to investigate variables within populate_and_search() with the interpreter.

francescalb · 2025-03-11T13:14:27Z

tests/backends/test_sparqlwrapper_graphdb_fuseki.py

    datasetinput = thisdir / "datadocumentation_sample.yaml"
    datasetinput2 = thisdir / "datadocumentation_sample2.yaml"

+    ts = get_triplestore(tsname)


I see no reason to do this

For debugging as explained above.

tripper/backends/sparqlwrapper.py

francescalb · 2025-03-11T13:19:25Z

tests/backends/test_sparqlwrapper_graphdb_fuseki.py

+    # Test INSERT query
+    query = """
+PREFIX : <http://example.com#>
+INSERT DATA {
+    :sub :pred :obj .
+}
+"""
+    EX = ts.bind("ex", "http://example.com#")
+    ts.update(query)
+    triples = list(ts.triples(EX.sub))
+    assert triples == [(EX.sub, EX.pred, EX.obj)]
+


I prefer that ts.update tests are done in the same place and not scattered around.

I don't want it to infer with the existing tests above, so I placed it at the end...

You already do by adding the delete at the top.

Would be better to use remove.

You already do by adding the delete at the top.

True. That is by purpose, to avoid strange failures due to garbage in the triplestore.

francescalb · 2025-03-11T13:20:37Z

tripper/backends/sparqlwrapper.py

+            the query() method for ASK/CONSTRUCT/SELECT/DESCRIBE queries.
+        """
+        query_type = self._get_sparql_query_type(update_object)
+        if query_type not in ("DELETE", "INSERT"):


If you put this in, I think you should also add a test to it. It is quite quick, there is already one that can be copied using query.

Added some tests for these as well

francescalb

Please view my comments. Also, this PR is not linked to an issue and why this should have priority now.

Co-authored-by: Francesca L. Bleken <48128015+francescalb@users.noreply.github.com>

…te-query

jesper-friis · 2025-03-11T13:56:48Z

Please view my comments. Also, this PR is not linked to an issue and why this should have priority now.

The main motivation for this PR is that it fixes some sporadic failing tests (like PR #324) by emptying the triplestore.

francescalb · 2025-03-12T07:13:52Z

Please view my comments. Also, this PR is not linked to an issue and why this should have priority now.

The main motivation for this PR is that it fixes some sporadic failing tests (like PR #324) by emptying the triplestore.
Are you sure 324 fails because if this? A new triplestore is added in each test.

jesper-friis · 2025-03-12T13:14:10Z

Please view my comments. Also, this PR is not linked to an issue and why this should have priority now.

The main motivation for this PR is that it fixes some sporadic failing tests (like PR #324) by emptying the triplestore.
Are you sure 324 fails because if this? A new triplestore is added in each test.

Yes, I think so. I got similar failures as in 324 in master locally, which I got rid of by cleaning the triplestore in the start of the test.

francescalb

Strongly displike these changes in the test, but one of us have to give.

Added update() method to sparqlwrapper backend.

3d892fc

jesper-friis requested a review from francescalb March 11, 2025 13:03

jesper-friis and others added 4 commits March 11, 2025 14:04

Clean up

d66f60f

Merge branch 'master' into update-query

890911e

Merge branch 'master' into update-query

21e5c33

Merge branch 'update-query' of github.com:EMMC-ASBL/tripper into upda…

8a65dbc

…te-query

francescalb reviewed Mar 11, 2025

View reviewed changes

tripper/backends/sparqlwrapper.py Outdated Show resolved Hide resolved

francescalb reviewed Mar 11, 2025

View reviewed changes

francescalb requested changes Mar 11, 2025

View reviewed changes

jesper-friis and others added 5 commits March 11, 2025 14:24

Update tripper/backends/sparqlwrapper.py

5c2fe0e

Co-authored-by: Francesca L. Bleken <48128015+francescalb@users.noreply.github.com>

Added annotation

c9fe6bf

Merge branch 'update-query' of github.com:EMMC-ASBL/tripper into upda…

2b4b24d

…te-query

Updated argument name

5a4ee71

Added disable to pylint

0de7c6b

jesper-friis requested a review from francescalb March 12, 2025 13:16

francescalb approved these changes Mar 14, 2025

View reviewed changes

jesper-friis merged commit 7450e6a into master Mar 14, 2025
19 checks passed

jesper-friis deleted the update-query branch March 14, 2025 15:33

Conversation

jesper-friis commented Mar 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of change

Checklist for the reviewer

Uh oh!

codecov bot commented Mar 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jesper-friis Mar 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

francescalb left a comment

Choose a reason for hiding this comment

Uh oh!

jesper-friis commented Mar 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

francescalb commented Mar 12, 2025

Uh oh!

jesper-friis commented Mar 12, 2025

Uh oh!

francescalb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jesper-friis commented Mar 11, 2025 •

edited

Loading

codecov bot commented Mar 11, 2025 •

edited

Loading

jesper-friis Mar 11, 2025 •

edited

Loading

jesper-friis commented Mar 11, 2025 •

edited

Loading