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

Iterate over dataset return quads #1382

Merged
merged 4 commits into from
Aug 28, 2021
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 21 additions & 0 deletions rdflib/graph.py
Original file line number Diff line number Diff line change
Expand Up @@ -1776,6 +1776,22 @@ class Dataset(ConjunctiveGraph):
rdflib.term.Literal("foo-bar"),
rdflib.term.URIRef("http://www.example.com/gr"))
>>>
>>> # which is equivalent to iterate over the dataset
>>> for q in ds: # doctest: +SKIP
... print(q) # doctest: +NORMALIZE_WHITESPACE
(rdflib.term.URIRef("http://example.org/a"),
rdflib.term.URIRef("http://www.example.org/b"),
rdflib.term.Literal("foo"),
None)
(rdflib.term.URIRef("http://example.org/x"),
rdflib.term.URIRef("http://example.org/y"),
rdflib.term.Literal("bar"),
rdflib.term.URIRef("http://www.example.com/gr"))
(rdflib.term.URIRef("http://example.org/x"),
rdflib.term.URIRef("http://example.org/z"),
rdflib.term.Literal("foo-bar"),
rdflib.term.URIRef("http://www.example.com/gr"))
>>>
>>> for q in ds.quads((None,None,None,g)): # doctest: +SKIP
... print(q) # doctest: +NORMALIZE_WHITESPACE
(rdflib.term.URIRef("http://example.org/x"),
Expand Down Expand Up @@ -1896,6 +1912,11 @@ def quads(self, quad):
else:
yield s, p, o, c.identifier

def __iter__(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

Note: I'm just some guy commenting and should not be considered to be someone with any authority here.

I'd recommend putting a type hint here so that the codebase gradually shifts towards full type hint / mypy support. I believe that the following would work:

from typing import Optional, Union

# For python < 3.9, else built-in tuple and collections.abc.Generator
from typing import Generator, Tuple

# Type aliases to make unpacking what's going on a little more human friendly
ContextNode = Union[BNode, URIRef]
DatasetQuad = Tuple[Node, URIRef, Node, Optional[ContextNode]]
...
def __iter__(self) -> Generator[DatasetQuad, None, None]:
...

I think that the alias that I suggested above for DatasetQuad may even be able to be made more specific. The usage of URIRef reflects the RDF specification that the properties of triples are all URI references. Analogous restrictions exist on the subjects (I believe that literals cannot be subjects, for example). It may be helpful to look into what the context field can be. I believe the context here can only be None, a BNode, or a URIRef, but it's been a little bit since I looked around.

Copy link
Member

Choose a reason for hiding this comment

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

I'd recommend putting a type hint here

Yes please! Our (the maintainers') unofficial policy is that type hinting should be present for all new RDFlib additions.

Copy link
Member

Choose a reason for hiding this comment

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

# Type aliases to make unpacking what's going on a little more human friendly

Cool!

"""Iterates over all quads in the store"""
return self.quads((None, None, None, None))



class QuotedGraph(Graph):
"""
Expand Down