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

Move Store API to work with identifiers, not graphs #409

Closed
wants to merge 70 commits into from

Conversation

gromgull
Copy link
Member

This is a long-time coming fix for #167.

There are two API layers in RDFLib, the Graph level and the Store level. The issue here is the handling of the fourth element of a quad, i.e. the context. Currently both layers use Graph objects to represent this fourth context element, but this leads to lots of trouble (see examples in #167).

This is an attempt to sort this out by having Graphs handled at the Graph API level, and have stores deal with the graph identifiers only. Hopefully, we can make this change transparent to end-users, but plugin developers will probably notice.

A list of changes:

  • all store.add/remove methods now raise an Exception if passed a graph.
  • the Graph API works on Graph objects and will pack/unpack as required.

This is not yet finished, but a PR helps me get an overview of what is left to fix.

@gromgull gromgull added this to the rdflib 5.0.0 milestone Jan 12, 2017
@gromgull gromgull force-pushed the identifier-as-context branch 2 times, most recently from 3db8ed3 to c1f1a14 Compare January 29, 2017 15:56
@coveralls
Copy link

coveralls commented Jan 29, 2017

Coverage Status

Coverage increased (+0.8%) to 63.596% when pulling c1f1a14 on gromgull:identifier-as-context into e3690e7 on RDFLib:master.

All add/remove methods now raise an Exception if passed a graph.
The Graph API works on Graph objects and will pack/unpack as required.
@gromgull
Copy link
Member Author

I've rebased this onto the six_2to3 branch, and it passes the tests now.

@gromgull
Copy link
Member Author

but the PR is for master 😅

@joernhees
Copy link
Member

one can change the base branch of a PR (edit on top), i'll do this...

@joernhees joernhees changed the base branch from master to six_2to3 January 29, 2017 20:02
@gromgull gromgull force-pushed the six_2to3 branch 4 times, most recently from 8f35c01 to 08e5cb8 Compare January 30, 2017 17:20
@gromgull gromgull changed the base branch from six_2to3 to master February 1, 2017 09:36
@nicholascar
Copy link
Member

I've assigned this to Milestone 6.0.0 and have marked it critical as this and graph IDing issues in general are well known annoyances and need a general solution (i.e. for Graph, ConjunctiveGraph, QuotedGraph??, Dataset and Store).

This PR needs deduplicating with PR #845.

@nicholascar
Copy link
Member

This PR is long since obsolete so I'm closing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants