Repository: cogent3/cogent3
Pull request #1878 (2024-05-28 07:39:48)

ENH: Improvements to the GFF parser


Patch:
diff --git a/changelog.d/20240528_173634_Gavin.Huttley.md b/changelog.d/20240528_173634_Gavin.Huttley.md
new file mode 100644
index 000000000..8d7a6b7ec
--- /dev/null
+++ b/changelog.d/20240528_173634_Gavin.Huttley.md
@@ -0,0 +1,43 @@
+<!--
+A new scriv changelog fragment.
+
+Uncomment the section that is right (remove the HTML comment wrapper).
+-->
+
+<!--
+### Contributors
+
+- A bullet item for the Contributors category.
+
+-->
+### ENH
+
+- Defined an abstract base class for Gff records and support users providing
+  their own class via the cogent3.parse.gff.gff_parser(make_record) argument.
+  This enables users to devise a custom implementation tuned to the features
+  of different provider gff data.
+
+<!--
+### BUG
+
+- A bullet item for the BUG category.
+
+-->
+<!--
+### DOC
+
+- A bullet item for the DOC category.
+
+-->
+<!--
+### Deprecations
+
+- A bullet item for the Deprecations category.
+
+-->
+<!--
+### Discontinued
+
+- A bullet item for the Discontinued category.
+
+-->
diff --git a/requirements.txt b/requirements.txt
index 5d188687d..f0bc64554 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -6,7 +6,7 @@ numba>0.48.0;python_version<'3.9'
 numba>0.53; python_version>='3.9'
 numba>0.54; python_version>='3.10'
 numba>=0.57.0; python_version=='3.11'
-scipy==1.13.1
+scipy==1.13.0
 nox==2024.4.15
 plotly==5.22.0
 typing_extensions>=3.7
diff --git a/src/cogent3/parse/gff.py b/src/cogent3/parse/gff.py
index 0aebed826..8e63e0a6d 100644
--- a/src/cogent3/parse/gff.py
+++ b/src/cogent3/parse/gff.py
@@ -7,6 +7,9 @@ import pathlib
 import re
 import typing
 
+from abc import ABC
+
+from cogent3.util import warning as c3warn
 from cogent3.util.io import PathType, open_
 
 
@@ -14,7 +17,7 @@ OptionalCallable = typing.Optional[typing.Callable]
 OptionalStrContainer = typing.Optional[typing.Union[str, typing.Sequence[str]]]
 OptionalIntList = typing.Optional[list[list[int]]]
 OptionalStr = typing.Optional[str]
-OptionalInt = typing.Optional[str]
+OptionalInt = typing.Optional[int]
 OptionalStrDict = typing.Optional[typing.Union[str, dict[str, str]]]
 OptionalBool = typing.Optional[bool]
 
@@ -25,12 +28,6 @@ def is_gff3(f) -> bool:
     raise TypeError(f"unsupported type {type(f)}")
 
 
-@is_gff3.register
-def _(f: pathlib.PurePath) -> bool:
-    with open_(f) as f:
-        return is_gff3(f)
-
-
 @is_gff3.register
 def _(f: pathlib.Path) -> bool:
     with open_(f) as f:
@@ -69,7 +66,55 @@ _gff_item_map = {
 }
 
 
-class GffRecord:
+class GffRecordABC(ABC):
+    """abstract base class for gff records"""
+
+    # attributes that need to be on the instance
+    __slots__ = (
+        "seqid",
+        "source",
+        "biotype",
+        "name",
+        "parent_id",
+        "start",
+        "stop",
+        "spans",
+        "strand",
+        "score",
+        "phase",
+        "attrs",
+        "comments",
+    )
+
+    def __repr__(self):
+        name = self.__class__.__name__
+        return (
+            f"{name}(seqid={self.seqid!r}, name={self.name!r}, "
+            f"biotype={self.biotype!r}, start={self.start}, stop={self.stop}, "
+            f"strand={self.strand!r}, spans={self.spans})"
+        )
+
+    def __getitem__(self, item: str):
+        item = item.lower()
+        return getattr(self, _gff_item_map.get(item, item))
+
+    def __setitem__(self, key: str, value: typing.Any):
+        key = key.lower()
+        setattr(self, _gff_item_map.get(key, key), value)
+
+    def update(self, values: dict[str, typing.Any]):
+        for key, value in values.items():
+            self[key] = value
+
+    def get(self, item: str) -> typing.Any:
+        item = item.lower()
+        return getattr(self, _gff_item_map.get(item, item), None)
+
+    def to_dict(self) -> dict[str, typing.Any]:
+        return {k: getattr(self, k) for k in self.__slots__}
+
+
+class GffRecord(GffRecordABC):
     """Container for single GFF record. Elements can be indexed as a dict
     or directly as attributes.
 
@@ -126,33 +171,6 @@ class GffRecord:
         self.comments = comments
         self.attrs = attrs
 
-    def __repr__(self):
-        name = self.__class__.__name__
-        return (
-            f"{name}(seqid={self.seqid!r}, name={self.name!r}, "
-            f"start={self.start}, stop={self.stop}, strand={self.strand!r}, "
-            f"spans={self.spans})"
-        )
-
-    def __getitem__(self, item: str):
-        item = item.lower()
-        return getattr(self, _gff_item_map.get(item, item))
-
-    def __setitem__(self, key: str, value: typing.Any):
-        key = key.lower()
-        setattr(self, _gff_item_map.get(key, key), value)
-
-    def update(self, values: dict[str, typing.Any]):
-        for key, value in values.items():
-            self[key] = value
-
-    def get(self, item: str) -> typing.Any:
-        item = item.lower()
-        return getattr(self, _gff_item_map.get(item, item), None)
-
-    def to_dict(self) -> dict[str, typing.Any]:
-        return {k: getattr(self, k) for k in self.__slots__}
-
 
 @functools.singledispatch
 def gff_parser(
@@ -160,7 +178,8 @@ def gff_parser(
     attribute_parser: OptionalCallable = None,
     seqids: OptionalStrContainer = None,
     gff3: OptionalBool = None,
-) -> typing.Iterable[GffRecord]:
+    make_record: OptionalCallable = None,
+) -> typing.Iterable[GffRecordABC]:
     """parses a gff file
 
     Parameters
@@ -175,41 +194,39 @@ def gff_parser(
         only gff records matching these sequence IDs are returned
     gff3
         True if the format is gff3
+    make_record
+        callable that constructs a GffRecordABC compatible record. Defaults to
+        GffRecord. Over-rides attribute_parser if provided.
 
     Returns
     -------
-    dict
-        contains each of the 9 parameters specified by gff3, and comments.
+    iterable of GffRecordABCs
     """
     gff3 = gff3 or is_gff3(f)
     yield from _gff_parser(
-        f, attribute_parser=attribute_parser, seqids=seqids, gff3=gff3
+        f,
+        attribute_parser=attribute_parser,
+        seqids=seqids,
+        gff3=gff3,
+        make_record=make_record,
     )
 
 
-@gff_parser.register
-def _(
-    f: pathlib.PurePath,
-    attribute_parser: OptionalCallable = None,
-    seqids: OptionalStrContainer = None,
-    gff3: OptionalBool = None,
-) -> typing.Iterable[GffRecord]:
-    with open_(f) as infile:
-        yield from gff_parser(
-            infile, attribute_parser=attribute_parser, seqids=seqids, gff3=gff3
-        )
-
-
 @gff_parser.register
 def _(
     f: pathlib.Path,
     attribute_parser: OptionalCallable = None,
     seqids: OptionalStrContainer = None,
     gff3: OptionalBool = None,
-) -> typing.Iterable[GffRecord]:
+    make_record: OptionalCallable = None,
+) -> typing.Iterable[GffRecordABC]:
     with open_(f) as infile:
         yield from gff_parser(
-            infile, attribute_parser=attribute_parser, seqids=seqids, gff3=gff3
+            infile,
+            attribute_parser=attribute_parser,
+            seqids=seqids,
+            gff3=gff3,
+            make_record=make_record,
         )
 
 
@@ -219,10 +236,15 @@ def _(
     attribute_parser: OptionalCallable = None,
     seqids: OptionalStrContainer = None,
     gff3: OptionalBool = None,
-) -> typing.Iterable[GffRecord]:
+    make_record: OptionalCallable = None,
+) -> typing.Iterable[GffRecordABC]:
     with open_(f) as infile:
         yield from gff_parser(
-            infile, attribute_parser=attribute_parser, seqids=seqids, gff3=gff3
+            infile,
+            attribute_parser=attribute_parser,
+            seqids=seqids,
+            gff3=gff3,
+            make_record=make_record,
         )
 
 
@@ -232,9 +254,14 @@ def _(
     attribute_parser: OptionalCallable = None,
     seqids: OptionalStrContainer = None,
     gff3: OptionalBool = None,
-) -> typing.Iterable[GffRecord]:
+    make_record: OptionalCallable = None,
+) -> typing.Iterable[GffRecordABC]:
     yield from _gff_parser(
-        f, attribute_parser=attribute_parser, seqids=seqids, gff3=gff3
+        f,
+        attribute_parser=attribute_parser,
+        seqids=seqids,
+        gff3=gff3,
+        make_record=make_record,
     )
 
 
@@ -243,13 +270,19 @@ def _gff_parser(
     attribute_parser: OptionalCallable = None,
     seqids: OptionalStrContainer = None,
     gff3: bool = True,
-) -> typing.Iterable[GffRecord]:
+    make_record: OptionalCallable = None,
+) -> typing.Iterable[GffRecordABC]:
     """parses a gff file"""
     seqids = seqids or set()
     seqids = {seqids} if isinstance(seqids, str) else set(seqids)
 
-    if attribute_parser is None:
+    if attribute_parser is None and make_record is None:
         attribute_parser = parse_attributes_gff3 if gff3 else parse_attributes_gff2
+    elif callable(make_record):
+        attribute_parser = None
+
+    if make_record is None:
+        make_record = GffRecord
 
     for line in f:
         # comments and blank lines
@@ -280,10 +313,9 @@ def _gff_parser(
         if start > end:
             start, end = end, start
 
-        # all attributes have an "ID" but this may not be unique
-        attributes = attribute_parser(attributes, (start, end))
+        attributes = attribute_parser(attributes) if attribute_parser else attributes
 
-        yield GffRecord(
+        yield make_record(
             seqid=seqid,
             source=source,
             biotype=type_,
@@ -297,7 +329,8 @@ def _gff_parser(
         )
 
 
-def parse_attributes_gff2(attributes: str, span: typing.Tuple[int, int]) -> dict:
+@c3warn.deprecated_args("2024.9", reason="not being used", discontinued="span")
+def parse_attributes_gff2(attributes: str) -> dict:
     """Returns a dict with name and info keys"""
     name = attributes[attributes.find('"') + 1 :]
     if '"' in name:
@@ -305,7 +338,8 @@ def parse_attributes_gff2(attributes: str, span: typing.Tuple[int, int]) -> dict
     return {"ID": name, "Info": attributes}
 
 
-def parse_attributes_gff3(attributes: str, span: typing.Tuple[int, int]) -> dict:
+@c3warn.deprecated_args("2024.9", reason="not being used", discontinued="span")
+def parse_attributes_gff3(attributes: str) -> dict:
     """Returns a dictionary containing all the attributes"""
     attributes = attributes.strip(";")
     attributes = attributes.split(";")
@@ -320,7 +354,9 @@ def parse_attributes_gff3(attributes: str, span: typing.Tuple[int, int]) -> dict
     return attributes
 
 
-def merged_gff_records(records: list[GffRecord], num_fake_ids: int) -> tuple[dict, int]:
+def merged_gff_records(
+    records: list[GffRecordABC], num_fake_ids: int
+) -> tuple[dict[str, GffRecordABC], int]:
     """merges GFF records that have the same ID
 
     Parameters
@@ -366,7 +402,6 @@ def merged_gff_records(records: list[GffRecord], num_fake_ids: int) -> tuple[dic
             reduced[record_id] = record
             reduced[record_id].spans = []
 
-        # should this just be an append?
         reduced[record_id].spans.append((record["start"], record["stop"]))
 
     del records
diff --git a/src/cogent3/util/warning.py b/src/cogent3/util/warning.py
index f4859b831..28e089477 100644
--- a/src/cogent3/util/warning.py
+++ b/src/cogent3/util/warning.py
@@ -1,7 +1,7 @@
 import functools
 import inspect
 
-from typing import Any, Callable, List, Optional, Tuple
+from typing import Any, Callable, Optional, Sequence, Tuple
 from warnings import catch_warnings, simplefilter
 from warnings import warn as _warn
 
@@ -65,8 +65,8 @@ _discontinued = discontinued  # renamed to avoid name clash with discontinued ar
 def deprecated_args(
     version: str,
     reason: str,
-    old_new: List[Tuple[str, str]] = None,
-    discontinued: List[str] = None,
+    old_new: Sequence[Tuple[str, str]] = None,
+    discontinued: Sequence[str] = None,
     stack_level=2,
 ) -> Callable[..., Any]:
     """


Test patch:
diff --git a/tests/test_parse/test_gff.py b/tests/test_parse/test_gff.py
index dac10325b..770bfa292 100644
--- a/tests/test_parse/test_gff.py
+++ b/tests/test_parse/test_gff.py
@@ -1,7 +1,9 @@
 """Unit tests for GFF and related parsers.
 """
 
+import io
 import os
+import pathlib
 import re
 
 from io import StringIO
@@ -10,7 +12,12 @@ from unittest import TestCase
 
 import pytest
 
-from cogent3.parse.gff import gff_parser, parse_attributes_gff2
+from cogent3.parse.gff import (
+    GffRecord,
+    gff_parser,
+    is_gff3,
+    parse_attributes_gff2,
+)
 
 
 headers = [
@@ -89,17 +96,14 @@ class GffTest(TestCase):
     def test_parse_attributes_gff2(self):
         """Test the parse_attributes_gff2 method"""
         self.assertEqual(
-            [
-                parse_attributes_gff2(x[1][8], (x[1][3], x[1][4]))["ID"]
-                for x in data_lines
-            ],
+            [parse_attributes_gff2(x[1][8])["ID"] for x in data_lines],
             ["HBA_HUMAN", "dJ102G20.C1.1", "", "BROADO5"],
         )
 
     def test_custom_attr_func(self):
         """user provided attr parser"""
         gff3_path = os.path.join("data/c_elegans_WS199_shortened_gff.gff3")
-        for result in gff_parser(gff3_path, attribute_parser=lambda x, y: x):
+        for result in gff_parser(gff3_path, attribute_parser=lambda x: x):
             self.assertIsInstance(result["Attributes"], str)
 
 
@@ -210,3 +214,80 @@ def test_gff_parser_headers():
             l.pop("attrs")
             got.append(set(l.values()))
         assert got == expect
+
+
+def test_is_gff3_invalid():
+    with pytest.raises(TypeError):
+        _ = is_gff3(b"blah")
+
+
+def make_gff_text():
+    return headers[0] + "".join([l for l, _ in data_lines])
+
+
+@pytest.mark.parametrize(
+    "source",
+    (
+        data_lines,
+        io.StringIO(make_gff_text()),
+        pathlib.Path("data/c_elegans_WS199_shortened_gff.gff3"),
+        "data/c_elegans_WS199_shortened_gff.gff3",
+    ),
+)
+def test_is_gfg3(source):
+    assert isinstance(is_gff3(source), bool)
+
+
+def test_repr_gff3_record():
+    rec = GffRecord(name="1")
+    assert "name='1'" in repr(rec)
+
+
+def test_gff3_record_get():
+    attrs = "some text"
+    rec = GffRecord(name="1", attrs=attrs)
+    assert rec.get("attributes") == attrs
+
+
+def test_gff3_record_set():
+    attrs = "some text"
+    rec = GffRecord(name="1")
+    rec.attrs = attrs
+    assert rec["attributes"] == attrs
+
+
+def test_gff3_record_update():
+    attrs = "some text"
+    rec = GffRecord(name="1")
+    rec.update({"attrs": attrs})
+    assert rec.get("attributes") == attrs
+
+
+@pytest.fixture
+def worm_path(DATA_DIR):
+    return DATA_DIR / "c_elegans_WS199_shortened_gff.gff3"
+
+
+@pytest.mark.parametrize("path_type", (str, pathlib.Path))
+def test_gff_parser_path_types(worm_path, path_type):
+    got = list(gff_parser(path_type(worm_path), gff3=True))
+    assert len(got) == 13
+
+
+@pytest.mark.parametrize(
+    "obj", (make_gff_text().splitlines(), io.StringIO(make_gff_text()))
+)
+def test_gff_parser_obj_types(obj):
+    got = list(gff_parser(obj))
+    assert len(got) == 4
+
+
+def test_gff_parser_make_record_override_attr_parser(worm_path):
+    got = list(
+        gff_parser(
+            worm_path, attribute_parser=lambda x: x.split(";"), make_record=GffRecord
+        )
+    )
+    assert len(got) == 13
+    # no split operation
+    assert isinstance(got[0].attrs, str)