Fixes #837 #1068
Fixes #837 #1068
Conversation
nilay77
commented
May 25, 2020
nilay77
commented
- Fixed Graph.subjects, Graph.predicates, and Graph.objects to return only unique values.
- Added test case for the above 3 methods
rdflib/graph.py
Outdated
| @@ -617,18 +617,27 @@ def set(self, triple): | |||
|
|
|||
| def subjects(self, predicate=None, object=None): | |||
| """A generator of subjects with the given predicate and object""" | |||
| subs = [] | |||
There was a problem hiding this comment.
Use sets for keeping track of seen (lists have O(n) lookup time).
| subs = [] | |
| subs = set() |
(And subsequently use subs.add(s)).
There was a problem hiding this comment.
@niklasl Oh yes, this makes sense. I'll update this. Are any other changes required?
There was a problem hiding this comment.
@niklasl I have pushed the suggested changes.
|
This cannot go into core as is because the memory usage will explode for large graphs. As mentioned in the discussion of #837 this is something that probably has to be implemented on the stores or under a different name, or with a different set of arguments e.g. |
|
@tgbugs I have added a parameter named unique to the current methods as suggested by you. I have also added exception handling code which suggests setting the parameter unique to False in case of a MemoryError. If you could give me an idea about how to produce a memory error I can even add a test case. |
| def subjects(self, predicate=None, object=None, unique=False): | ||
| """A generator of unique subjects with the given predicate and object""" | ||
|
|
||
| if(unique==False): |
| def predicates(self, subject=None, object=None, unique=False): | ||
| """A generator of unique predicates with the given subject and object""" | ||
|
|
||
| if(unique==False): |
| def objects(self, subject=None, predicate=None, unique=False): | ||
| """A generator of unique objects with the given subject and predicate""" | ||
|
|
||
| if(unique==False): |
There was a problem hiding this comment.
-
Please change the 3 x if statements in the code to use Python style, as indicated by comments
-
You've not provided any tests for different outcomes based on whether or not
uniqueis set. You need a test for non-unique triples (uniquenot set) v. unique triples (uniqueset).
(you may have fun implementing a test to show non-unique values since the default store for Graph() auto-deduplicates triples anyway!)
|
Closing this PR as the proponent hasn't addressed comments in a couple of months. @nilay77 please reopen if you want to progress this, but you will need to address comments. |
ghost
mentioned this pull request
