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

SPARQLStore customizable BNode handling #513

Merged
merged 6 commits into from
Aug 27, 2015

Conversation

joernhees
Copy link
Member

this implements the discussion in #512

  • deprecates CastToTerm, TraverseSPARQLResultDOM and localName methods
  • deprecates SPARQLStore(..., bNodeAsURI) argument
  • adds two SPARQLStore(..., node_to_sparql, node_from_result) args to customize node transformations
  • added a doc example how to transform BNodes
  • several code cleanups

please check

ssssam and others added 2 commits August 22, 2015 20:33
The 4Store triple store understands <bnode:> URIs as pointing to blank
nodes, which makes it possible to query and update them using its SPARQL
endpoint.
…ocalName methods

also prepared configurable node_from_result arg.
def _node_to_sparql(node):
if isinstance(node, BNode):
raise Exception(
"SPARQLStore does not support BNodes! "
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe this Exception should now say "SPARQLStore does not support BNodes by default! See ..."

Copy link
Member Author

Choose a reason for hiding this comment

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

i thought so too, but as this currently raises an overly broad exception, the only thing people can do is rely on its message to actually handle it. That's why I decided against it for now and didn't change the message.

i made an issue to change all raise Exception to something more appropriate in 5.0.0 (see #515)

@uholzer
Copy link
Contributor

uholzer commented Aug 25, 2015

This has been quite some work it seems. Except for the nitpick in my previous comment, this looks good to me.

@joernhees
Copy link
Member Author

eh, it's mostly moves, deprecations and cleanups...

thanks for reviewing, i'll merge this soon if no one objects

@ssssam
Copy link
Contributor

ssssam commented Aug 26, 2015

This looks like it'll resolve the issue I had in #511 and #512, thanks !!

joernhees added a commit that referenced this pull request Aug 27, 2015
SPARQLStore customizable BNode handling, deprecations and cleanups
@joernhees joernhees merged commit cb6efeb into RDFLib:master Aug 27, 2015
@joernhees joernhees deleted the sparql_store_bnode branch August 27, 2015 08:00
joernhees added a commit that referenced this pull request Aug 27, 2015
* sparql_store_bnode:
  test changed to use _node_from_result instead of CastToTerm
@joernhees
Copy link
Member Author

@ssssam you can try by installing rdflib like this: pip install -U git+git://github.com/RDFLib/rdflib (until 4.2.2 is released)

joernhees added a commit to joernhees/rdflib that referenced this pull request Mar 9, 2016
initBindings, contexts, addN, remove, add_graph and remove_graph now call the
node_to_sparql customizable function. Some support for BNode graph names added.

Add-on for RDFLib#513, see also RDFLib#511, RDFLib#512
joernhees added a commit to joernhees/rdflib that referenced this pull request Mar 9, 2016
query (initBindings), contexts, addN, remove, add_graph and remove_graph now call
node_to_sparql. Some support for BNode graph names added.

Add-on for RDFLib#513, see also RDFLib#511, RDFLib#512
joernhees added a commit to joernhees/rdflib that referenced this pull request Mar 9, 2016
query (initBindings), contexts, addN, remove, add_graph and remove_graph now call
node_to_sparql. Some support for BNode graph names added.

Add-on for RDFLib#513, see also RDFLib#511, RDFLib#512
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request in-resolution SPARQL store Related to a store.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants