Update subset handling (#476)

This plagues the ChEBI parsing since there are thousands of `2:STAR` and
3:STAR` values for subsets, which are not actually CURIEs but somehow
get serialized this way due to how OWLAPI works

Author: cthoyt

src/pyobo/getters.py

@@ -144,6 +144,7 @@ def get_ontology(
 
     if version is None:
         version = _get_version_from_artifact(prefix)
+        logger.info(f"[%s] current version is {version}", prefix)
 
     if force_process:
         obonet_json_gz_path = None

src/pyobo/struct/functional/obo_to_functional.py

@@ -69,7 +69,7 @@ def get_ontology_axioms(obo_ontology: Obo) -> Iterable[f.Box]:
 
     if obo_ontology.subsetdefs:
         yield f.Declaration("oboInOwl:SubsetProperty", type="AnnotationProperty")
-        for subset_typedef, subset_label in obo_ontology.subsetdefs:
+        for subset_typedef, subset_label in obo_ontology.subsetdefs.items():
             yield f.Declaration(subset_typedef, type="AnnotationProperty")
             yield m.LabelMacro(subset_typedef, subset_label)
             yield f.SubAnnotationPropertyOf(subset_typedef, "oboInOwl:SubsetProperty")

src/pyobo/struct/obo/reader.py

@@ -10,7 +10,7 @@
 from io import StringIO
 from pathlib import Path
 from textwrap import dedent
-from typing import Any
+from typing import Any, TypeAlias
 
 import bioregistry
 import networkx as nx
@@ -37,8 +37,8 @@
     SynonymTypeDef,
     Term,
     TypeDef,
+    build_ontology,
     default_reference,
-    make_ad_hoc_ontology,
 )
 from ..struct_utils import Annotation, Stanza
 from ..typedef import comment as has_comment
@@ -174,7 +174,7 @@ def from_obonet(
 
     missing_typedefs: set[ReferenceTuple] = set()
 
-    subset_typedefs = _get_subsetdefs(graph.graph, ontology_prefix=ontology_prefix)
+    subset_typedefs = _get_subsetdefs(graph.graph, ontology_prefix=ontology_prefix, strict=strict)
 
     root_terms: list[Reference] = []
     property_values: list[Annotation] = []
@@ -243,20 +243,22 @@ def from_obonet(
         use_tqdm=use_tqdm,
     )
 
-    return make_ad_hoc_ontology(
-        _ontology=ontology_prefix,
-        _name=name,
-        _auto_generated_by=graph.graph.get("auto-generated-by"),
-        _typedefs=list(typedefs.values()),
-        _synonym_typedefs=list(synonym_typedefs.values()),
-        _date=date,
-        _data_version=data_version,
-        _root_terms=root_terms,
+    return build_ontology(
+        prefix=ontology_prefix,
+        name=name,
+        auto_generated_by=graph.graph.get("auto-generated-by"),
+        typedefs=list(typedefs.values()),
+        synonym_typedefs=list(synonym_typedefs.values()),
+        date=date,
+        version=data_version,
+        idspaces=idspaces,
+        root_terms=root_terms,
+        subsetdefs=subset_typedefs,
+        properties=property_values,
+        imports=imports,
+        # ontology_iri
+        # ontology_version_iri
         terms=terms,
-        _property_values=property_values,
-        _subsetdefs=subset_typedefs,
-        _imports=imports,
-        _idspaces=idspaces,
     )
 
 
@@ -268,7 +270,7 @@ def _get_terms(
     upgrade: bool,
     typedefs: Mapping[ReferenceTuple, TypeDef],
     synonym_typedefs: Mapping[ReferenceTuple, SynonymTypeDef],
-    subset_typedefs,
+    subset_typedefs: SubsetTypeDefs,
     missing_typedefs: set[ReferenceTuple],
     macro_config: MacroConfig,
     use_tqdm: bool = False,
@@ -332,7 +334,13 @@ def _get_terms(
             missing_typedefs=missing_typedefs,
         )
         _process_replaced_by(term, data, ontology_prefix=ontology_prefix, strict=strict)
-        _process_subsets(term, data, ontology_prefix=ontology_prefix, strict=strict)
+        _process_subsets(
+            term,
+            data,
+            ontology_prefix=ontology_prefix,
+            strict=strict,
+            subset_typedefs=subset_typedefs,
+        )
         _process_intersection_of(term, data, ontology_prefix=ontology_prefix, strict=strict)
         _process_union_of(term, data, ontology_prefix=ontology_prefix, strict=strict)
         _process_equivalent_to(term, data, ontology_prefix=ontology_prefix, strict=strict)
@@ -379,13 +387,19 @@ def _process_creation_date(term: Stanza, data) -> None:
 
 def _process_union_of(term: Stanza, data, *, ontology_prefix: str, strict: bool) -> None:
     for reference in iterate_node_reference_tag(
-        "union_of", data=data, ontology_prefix=ontology_prefix, strict=strict, node=term.reference
+        term,
+        "union_of",
+        data=data,
+        ontology_prefix=ontology_prefix,
+        strict=strict,
+        node=term.reference,
     ):
         term.append_union_of(reference)
 
 
 def _process_equivalent_to(term: Stanza, data, *, ontology_prefix: str, strict: bool) -> None:
     for reference in iterate_node_reference_tag(
+        term,
         "equivalent_to",
         data=data,
         ontology_prefix=ontology_prefix,
@@ -397,6 +411,7 @@ def _process_equivalent_to(term: Stanza, data, *, ontology_prefix: str, strict:
 
 def _process_disjoint_from(term: Stanza, data, *, ontology_prefix: str, strict: bool) -> None:
     for reference in iterate_node_reference_tag(
+        term,
         "disjoint_from",
         data=data,
         ontology_prefix=ontology_prefix,
@@ -408,15 +423,15 @@ def _process_disjoint_from(term: Stanza, data, *, ontology_prefix: str, strict:
 
 def _process_alts(term: Stanza, data, *, ontology_prefix: str, strict: bool) -> None:
     for alt_reference in iterate_node_reference_tag(
-        "alt_id", data, node=term.reference, strict=strict, ontology_prefix=ontology_prefix
+        term, "alt_id", data, node=term.reference, strict=strict, ontology_prefix=ontology_prefix
     ):
         term.append_alt(alt_reference)
 
 
 def _process_parents(term: Stanza, data, *, ontology_prefix: str, strict: bool) -> None:
     for tag in ["is_a", "instance_of"]:
         for parent in iterate_node_reference_tag(
-            tag, data, node=term.reference, strict=strict, ontology_prefix=ontology_prefix
+            term, tag, data, node=term.reference, strict=strict, ontology_prefix=ontology_prefix
         ):
             term.append_parent(parent)
 
@@ -512,20 +527,35 @@ def _process_relations(
 
 def _process_replaced_by(stanza: Stanza, data, *, ontology_prefix: str, strict: bool) -> None:
     for reference in iterate_node_reference_tag(
-        "replaced_by", data, node=stanza.reference, strict=strict, ontology_prefix=ontology_prefix
+        stanza,
+        "replaced_by",
+        data,
+        node=stanza.reference,
+        strict=strict,
+        ontology_prefix=ontology_prefix,
     ):
         stanza.append_replaced_by(reference)
 
 
-def _process_subsets(stanza: Stanza, data, *, ontology_prefix: str, strict: bool) -> None:
+UNDEFINED_SUBSETS = set()
+
+
+def _process_subsets(
+    stanza: Stanza, data, *, ontology_prefix: str, strict: bool, subset_typedefs: SubsetTypeDefs
+) -> None:
     for reference in iterate_node_reference_tag(
+        stanza,
         "subset",
         data,
         node=stanza.reference,
         strict=strict,
         ontology_prefix=ontology_prefix,
         counter=SUBSET_ERROR_COUNTER,
     ):
+        if reference not in subset_typedefs:
+            if reference not in UNDEFINED_SUBSETS:
+                logger.warning("[%s] undefined subset: %s", stanza.curie, reference)
+                UNDEFINED_SUBSETS.add(reference)
         stanza.append_subset(reference)
 
 
@@ -669,8 +699,13 @@ def _handle_xref(
 SUBSET_ERROR_COUNTER: Counter[tuple[str, str]] = Counter()
 
 
-def _get_subsetdefs(graph: nx.MultiDiGraph, ontology_prefix: str) -> list[tuple[Reference, str]]:
-    rv = []
+SubsetTypeDefs: TypeAlias = dict[Reference, str]
+
+
+def _get_subsetdefs(
+    graph: nx.MultiDiGraph, ontology_prefix: str, *, strict: bool = False
+) -> SubsetTypeDefs:
+    rv = {}
     for subsetdef in graph.get("subsetdef", []):
         left, _, right = subsetdef.partition(" ")
         if not right:
@@ -682,11 +717,12 @@ def _get_subsetdefs(graph: nx.MultiDiGraph, ontology_prefix: str) -> list[tuple[
             name=right,
             line=subsetdef,
             counter=SUBSET_ERROR_COUNTER,
+            strict=strict,
         )
         if left_ref is None:
             continue
         right = right.strip('"')
-        rv.append((left_ref, right))
+        rv[left_ref] = right
     return rv
 
 
@@ -812,6 +848,7 @@ def iterate_typedefs(
     # can't really have a pre-defined set of synonym typedefs here!
     synonym_typedefs: Mapping[ReferenceTuple, SynonymTypeDef] = {}
     typedefs: Mapping[ReferenceTuple, TypeDef] = {}
+    subset_typedefs: SubsetTypeDefs = {}  # FIXME
     missing_typedefs: set[ReferenceTuple] = set()
     for data in graph.graph.get("typedefs", []):
         if "id" in data:
@@ -889,7 +926,13 @@ def iterate_typedefs(
             missing_typedefs=missing_typedefs,
         )
         _process_replaced_by(typedef, data, ontology_prefix=ontology_prefix, strict=strict)
-        _process_subsets(typedef, data, ontology_prefix=ontology_prefix, strict=strict)
+        _process_subsets(
+            typedef,
+            data,
+            ontology_prefix=ontology_prefix,
+            strict=strict,
+            subset_typedefs=subset_typedefs,
+        )
         _process_intersection_of(typedef, data, ontology_prefix=ontology_prefix, strict=strict)
         _process_union_of(typedef, data, ontology_prefix=ontology_prefix, strict=strict)
         _process_equivalent_to(typedef, data, ontology_prefix=ontology_prefix, strict=strict)
@@ -904,6 +947,7 @@ def iterate_typedefs(
         _process_holds_over_chain(typedef, data, ontology_prefix=ontology_prefix, strict=strict)
         typedef.disjoint_over.extend(
             iterate_node_reference_tag(
+                typedef,
                 "disjoint_over",
                 data,
                 node=typedef.reference,
@@ -913,6 +957,7 @@ def iterate_typedefs(
         )
         typedef.transitive_over.extend(
             iterate_node_reference_tag(
+                typedef,
                 "transitive_over",
                 data,
                 node=typedef.reference,
@@ -926,6 +971,7 @@ def iterate_typedefs(
 
 def _process_consider(stanza: Stanza, data, *, ontology_prefix: str, strict: bool = False):
     for reference in iterate_node_reference_tag(
+        stanza,
         "consider",
         data,
         node=stanza.reference,
@@ -1349,6 +1395,7 @@ def _parse_default_prop(property_id, ontology_prefix) -> Reference | None:
 
 
 def iterate_node_reference_tag(
+    stanza: Stanza,
     tag: str,
     data: Mapping[str, Any],
     *,
@@ -1359,21 +1406,35 @@ def iterate_node_reference_tag(
     counter: Counter[tuple[str, str]] | None = None,
 ) -> Iterable[Reference]:
     """Extract a list of CURIEs from the data."""
-    for identifier in data.get(tag, []):
+    for str_or_curie_or_uri in data.get(tag, []):
         reference = _obo_parse_identifier(
-            identifier,
+            str_or_curie_or_uri,
             strict=strict,
             node=node,
             ontology_prefix=ontology_prefix,
             upgrade=upgrade,
             counter=counter,
         )
-        if reference is None:
+        if reference is not None:
+            yield reference
+        elif tag == "subset":
+            # this is to avoid the millions of 2:STAR and 3:STAR errors when parsing ChEBI that makes
+            # it take forever. In general, most of the subset identifiers are totally borked.
+            if str_or_curie_or_uri not in SUBSET_INVALIDS:
+                logger.warning(
+                    "[%s] %s - could not parse subset identifier: %s",
+                    stanza.curie,
+                    tag,
+                    str_or_curie_or_uri,
+                )
+                SUBSET_INVALIDS.add(str_or_curie_or_uri)
+        else:
             logger.warning(
-                "[%s] %s - could not parse identifier: %s", ontology_prefix, tag, identifier
+                "[%s] %s - could not parse identifier: %s", stanza.curie, tag, str_or_curie_or_uri
             )
-        else:
-            yield reference
+
+
+SUBSET_INVALIDS: set[str] = set()
 
 
 def _process_intersection_of(

src/pyobo/struct/struct.py

@@ -600,7 +600,7 @@ class Obo:
     #: A cache of terms
     _items: list[Term] | None = field(init=False, default=None, repr=False)
 
-    subsetdefs: ClassVar[list[tuple[Reference, str]] | None] = None
+    subsetdefs: ClassVar[dict[Reference, str] | None] = None
 
     property_values: ClassVar[list[Annotation] | None] = None
 
@@ -696,7 +696,7 @@ def _get_prefixes(self) -> set[str]:
             prefixes.update(stanza._get_prefixes())
         for synonym_typedef in self.synonym_typedefs or []:
             prefixes.update(synonym_typedef._get_prefixes())
-        prefixes.update(subset.prefix for subset, _ in self.subsetdefs or [])
+        prefixes.update(subset.prefix for subset in self.subsetdefs or [])
         # _iterate_property_pairs covers metadata, root terms,
         # and properties in self.property_values
         prefixes.update(_get_prefixes_from_annotations(self._iterate_property_pairs()))
@@ -711,7 +711,7 @@ def _get_references(self) -> dict[str, set[Reference]]:
         for rr in itt.chain(self, self.typedefs or [], self.synonym_typedefs or []):
             for prefix, references in rr._get_references().items():
                 rv[prefix].update(references)
-        for subset, _ in self.subsetdefs or []:
+        for subset in self.subsetdefs or {}:
             rv[subset.prefix].add(subset)
         # _iterate_property_pairs covers metadata, root terms,
         # and properties in self.property_values
@@ -881,7 +881,7 @@ def iterate_obo_lines(
         for imp in self.imports or []:
             yield f"import: {imp}"
         # 7
-        for subset, subset_remark in self.subsetdefs or []:
+        for subset, subset_remark in (self.subsetdefs or {}).items():
             yield f'subsetdef: {reference_escape(subset, ontology_prefix=self.ontology)} "{subset_remark}"'
         # 8
         for synonym_typedef in sorted(self.synonym_typedefs or []):
@@ -2400,7 +2400,7 @@ def build_ontology(
     version: str | None = None,
     idspaces: dict[str, str] | None = None,
     root_terms: list[Reference] | None = None,
-    subsetdefs: list[tuple[Reference, str]] | None = None,
+    subsetdefs: dict[Reference, str] | None = None,
     properties: list[Annotation] | None = None,
     imports: list[str] | None = None,
     description: str | None = None,
@@ -2410,6 +2410,8 @@ def build_ontology(
     repository: str | None = None,
     ontology_iri: str | None = None,
     ontology_version_iri: str | None = None,
+    auto_generated_by: str | None = None,
+    date: datetime.datetime | None = None,
 ) -> Obo:
     """Build an ontology from parts."""
     if name is None:
@@ -2459,10 +2461,10 @@ def build_ontology(
     return make_ad_hoc_ontology(
         _ontology=prefix,
         _name=name,
-        # _auto_generated_by
+        _auto_generated_by=auto_generated_by,
         _typedefs=typedefs,
         _synonym_typedefs=synonym_typedefs,
-        # _date: datetime.datetime | None = None,
+        _date=date,
         _data_version=version,
         _idspaces=idspaces,
         _root_terms=root_terms,
@@ -2485,7 +2487,7 @@ def make_ad_hoc_ontology(
     _data_version: str | None = None,
     _idspaces: Mapping[str, str] | None = None,
     _root_terms: list[Reference] | None = None,
-    _subsetdefs: list[tuple[Reference, str]] | None = None,
+    _subsetdefs: dict[Reference, str] | None = None,
     _property_values: list[Annotation] | None = None,
     _imports: list[str] | None = None,
     _ontology_iri: str | None = None,