Skip to content

Commit

Permalink
Merge pull request #41 from TheJacksonLaboratory/type-annotations
Browse files Browse the repository at this point in the history
Fix `OntologyGraph` type annotations - breaking change
  • Loading branch information
ielis authored Sep 1, 2023
2 parents d50990e + ee64f1c commit 99a212e
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 18 deletions.
19 changes: 10 additions & 9 deletions docs/use-hierarchy.rst
Original file line number Diff line number Diff line change
Expand Up @@ -81,17 +81,18 @@ Augmenting term with ancestors/descendants

TODO - show ancestors/descendants with `include_source`.

.. note::
.. _iterable-vs-iterator:

**Iterables vs. collections**
Iterators vs. collections
^^^^^^^^^^^^^^^^^^^^^^^^^

You may have noticed the looping in the previous examples. The API does *not* promise a `set`, `list`, or any
other collection and it provides :class:`typing.Iterable` instead.
Therefore, the ontology graph implementation may choose to return a lazily evaluated iterable implementation.
You may have noticed the looping in the previous examples. The API does *not* promise a `set`, `list`, or any
other collection and it provides :class:`typing.Iterator` instead.
Therefore, the ontology graph implementation may choose to return a lazily evaluated iterable implementation.

Lazy iterables have pros and cons. Thanks to the lazy evaluation, we do not need to calculate the entire
ancestor/descendant set if all we need is to find one of the terms.
On the flip side, we need to *"collect"* the iterable into a list/set if that's what we're really after,
incurring unnecessary creation of a new collection.
The iterators have pros and cons. Thanks to the lazy evaluation, we do not need to calculate the entire
ancestor/descendant set if all we need is to find one of the terms.
On the flip side, we need to *"collect"* the iterator into a list/set if that's what we're really after,
incurring unnecessary creation of a new collection.

.. TODO - move tutorial parts here.
23 changes: 14 additions & 9 deletions src/hpotk/graph/_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@ class OntologyGraph(typing.Generic[NODE], metaclass=abc.ABCMeta):
The graph is generic over a node type which must extend :class:`TermId`.
The graph must not be empty, it must consist of at least one node.
.. note::
`OntologyGraph` provides **iterators** for traversals instead of sets, lists, etc.
See :ref:`iterable-vs-iterator` to learn why.
"""

@property
Expand All @@ -28,9 +33,9 @@ def root(self) -> NODE:

@abc.abstractmethod
def get_children(self, source: typing.Union[str, NODE, Identified],
include_source: bool = False) -> typing.Iterable[NODE]:
include_source: bool = False) -> typing.Iterator[NODE]:
"""
Get an iterable with the children of the `source` node.
Get an iterator with the children of the `source` node.
:param source: a :class:`TermId`, an item that *has* a :class:`TermId` (:class:`Identified`), or a curie `str`
representing the source node.
Expand All @@ -41,9 +46,9 @@ def get_children(self, source: typing.Union[str, NODE, Identified],

@abc.abstractmethod
def get_descendants(self, source: typing.Union[str, NODE, Identified],
include_source: bool = False) -> typing.Iterable[NODE]:
include_source: bool = False) -> typing.Iterator[NODE]:
"""
Get an iterable with the descendants of the `source` node.
Get an iterator with the descendants of the `source` node.
:param source: a :class:`TermId`, an item that *has* a :class:`TermId` (:class:`Identified`), or a curie `str`
representing the source node.
Expand All @@ -54,9 +59,9 @@ def get_descendants(self, source: typing.Union[str, NODE, Identified],

@abc.abstractmethod
def get_parents(self, source: typing.Union[str, NODE, Identified],
include_source: bool = False) -> typing.Iterable[NODE]:
include_source: bool = False) -> typing.Iterator[NODE]:
"""
Get an iterable with the parents of the `source` node.
Get an iterator with the parents of the `source` node.
:param source: a :class:`TermId`, an item that *has* a :class:`TermId` (:class:`Identified`), or a curie `str`
representing the source node.
Expand All @@ -67,9 +72,9 @@ def get_parents(self, source: typing.Union[str, NODE, Identified],

@abc.abstractmethod
def get_ancestors(self, source: typing.Union[str, NODE, Identified],
include_source: bool = False) -> typing.Iterable[NODE]:
include_source: bool = False) -> typing.Iterator[NODE]:
"""
Get an iterable with the ancestors of the `source` node.
Get an iterator with the ancestors of the `source` node.
:param source: a :class:`TermId`, an item that *has* a :class:`TermId` (:class:`Identified`), or a curie `str`
representing the source node.
Expand Down Expand Up @@ -138,7 +143,7 @@ def is_descendant_of(self, sub: typing.Union[str, NODE, Identified],
return self._run_query(self.get_descendants, sub, obj)

@staticmethod
def _run_query(func: typing.Callable[[NODE], typing.Iterable[NODE]],
def _run_query(func: typing.Callable[[NODE], typing.Iterator[NODE]],
sub: typing.Union[str, NODE, Identified],
obj: typing.Union[str, NODE, Identified]) -> bool:
sub = OntologyGraph._map_to_term_id(sub)
Expand Down
8 changes: 8 additions & 0 deletions src/hpotk/graph/_test__api.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import typing
import unittest

import ddt
Expand Down Expand Up @@ -190,6 +191,13 @@ def test_query_methods_with_unknown_source(self, func_name):
func(existing, unknown)
self.assertEqual('Term ID not found in the graph: HP:999', ctx.exception.args[0])

def test_traversal_methods_produce_iterators(self):
whatever = TermId.from_curie('HP:1')
self.assertIsInstance(self.GRAPH.get_parents(whatever), typing.Iterator)
self.assertIsInstance(self.GRAPH.get_children(whatever), typing.Iterator)
self.assertIsInstance(self.GRAPH.get_ancestors(whatever), typing.Iterator)
self.assertIsInstance(self.GRAPH.get_descendants(whatever), typing.Iterator)


class SimpleIdentified(Identified):

Expand Down

0 comments on commit 99a212e

Please sign in to comment.