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

SPARQL result parsing #2796

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

white-gecko
Copy link
Member

@white-gecko white-gecko commented Jun 10, 2024

Summary of changes

  • Remove plugin registration for "application/sparql-results+xml; charset=UTF-8"
  • Resolve TODO: "Pull in these from the result implementation plugins?" for _response_mime_types in sparqlconnector
    • The _response_mime_types dict is replaced by a method response_mime_types that generates an Accept filed containing the supported mime types. Per default these are all mime types for which ResultParser plugins are registered.
  • Dynamically register the rdflib.plugins.sparql.results.graph.GraphResultParser plugin for all names of the Parser plugins since it calls the Parsers internally.

Eventually, these changes could be superseded by a more flexible plugin system, that would distinguish between plugin name, supported format, and supported mime-type.

The changes should maintain backward compatibility.

But this API is now more flexible. The returnFormat for SPARQLConnector and SPARQLStore is now optional and None per default. Also the default Accept header lists all mime-types supported by parsers. Maybe this should be mentioned in the change notes.

Checklist

  • Checked that there aren't other open pull requests for
    the same change.
  • Checked that all tests and type checking passes.
  • Considered granting push permissions to the PR branch,
    so maintainers can fix minor issues and keep your PR up to date.

…et=UTF-8"

Remove ResultParser plugin registration with the name "application/sparql-results+xml; charset=UTF-8" since it is never called in internal code.
The only place where ResultParser plugins are requested is in "rdflib/query.py:276"

```
if format:
    plugin_key = format
elif content_type:
    plugin_key = content_type.split(";", 1)[0]
else:
    plugin_key = "xml"

parser = plugin.get(plugin_key, ResultParser)()
```

In case of a content-type the charset is split off already.
@white-gecko white-gecko marked this pull request as draft June 10, 2024 15:36
@white-gecko white-gecko marked this pull request as ready for review June 11, 2024 08:46
@ashleysommer
Copy link
Contributor

@white-gecko this looks great, thanks. This addresses an issue I had only last month, I was trying to dynamically get the correct SPARQL result parser based on a mimetype, though the RDFLib plugin system, and I couldn't get it to work. Looks like now there is a way to do it.

@ashleysommer
Copy link
Contributor

@white-gecko I've clearned up the commit pipeline, so we don't have Dependabot conflicts, mypy issues, black updates and ruff updates in the loop to prevent the proper testing of PRs. I've just re-ran the test suite on your PR, and it is failing one real test:

test_example[sparqlstore_example.py]

@nicholascar nicholascar changed the title Feature/sparql result parsing SPARQL result parsing Jul 24, 2024
@nicholascar nicholascar added enhancement New feature or request awaiting feedback More feedback is needed from the author of the PR or Issue. labels Jul 24, 2024
@white-gecko
Copy link
Member Author

white-gecko commented Jul 26, 2024

@ashleysommer I'm sorry, I don't really understand the test output, since it is successful for some runs but the output of the other test doesn't show the failing test.

Ok, I got it, these are failing:

    graph = Graph("SPARQLStore", identifier="http://dbpedia.org")
    graph.open("http://dbpedia.org/sparql")

    pop = graph.value(URIRef("http://dbpedia.org/resource/Berlin"), dbo.populationTotal)

This can be due to the content type, mimetype handling.

@ashleysommer
Copy link
Contributor

Weirdly I'm seeing that exact same test_example[sparqlstore_example.py] failure on this unrelated PR: #2818
Do you know what could be causing that?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting feedback More feedback is needed from the author of the PR or Issue. enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants