Biopython: Add type annotations

[sbliven]

I would like to propose introducing type annotations to BioPython.

For those unfamiliar, type annotations are specified by PEP 484 and add static type-checking to python code. Two syntaxes are available; an annotation syntax for python 3 and a type comment syntax which is compatible with python 2 code. For example:

def f(num1, my_float=3.5):
    # type: (int, float) -> float
    return num1 + my_float

The annotations are no-ops at runtime, but allow type checking tools to be run over the code (e.g. as a tox test) to detect type errors. The python community has standardized on the MyPy type checking package for the static analysis.

Code without a type annotation is treated permissively by the type checker, so types can be added incrementally to the project over time.

BioPython could start providing types for some core data structures using the py27 comment syntax. At whatever point BioPython drops 2.7 support (after 2020), development could then switch to using the cleaner annotation syntax.

[peterjc]

Sounds good in principle.

I would be inclined to wait until early 2020 when we drop Python 2 support, so as to ignore the Python 2 legacy comment syntax, and start directly with the annotation syntax.

[sbliven]

It should be very straightforward to switch between syntaxes, so I'm not sure what we gain by waiting.

I would like to type the PDB package as I work on the MolQL module (this is coming along nicely btw). However I'm happy to add the type comments in a branch and rebase in 2020.

[peterjc]

Well if you have a pressing need, and don't mind dealing with the syntax update, why not have a shot at it for Bio.PDB?

[sbliven]

Sounds good!

[peterjc]

We have now dropped Python 2 support, and target only Python 3.6+ - so that isn't an obstacle now.

[chris-rands]

There are some tools for helping with automatic type hinting/variable annotation. I'm not familiar with any of them and I think it will require careful review and manual work, but here's an example of a quick test I did with Instagram's monkeytype. You can see at least that automation might kick-start the process:

diff --git a/Bio/Seq.py b/Bio/Seq.py
index 71818ec89..43bb4a85e 100644
--- a/Bio/Seq.py
+++ b/Bio/Seq.py
@@ -35,6 +35,10 @@ from Bio.Data.IUPACData import ambiguous_rna_letters as _ambiguous_rna_letters
 from Bio.Data import CodonTable


+from Bio.Alphabet import Alphabet
+from Bio.Data.CodonTable import AmbiguousCodonTable
+from array import array
+from typing import List, Optional, Tuple, Union
 def _maketrans(complement_mapping):
     """Make a python string translation table (PRIVATE).

@@ -303,7 +307,7 @@ class Seq:
             # Return the (sub)sequence as another Seq object
             return Seq(self._data[index], self.alphabet)

-    def __add__(self, other):
+    def __add__(self, other: Union[Seq, str]) -> Seq:
         """Add another sequence or string to this sequence.

         If adding a string to a Seq, the alphabet is preserved:
@@ -375,7 +379,7 @@ class Seq:
         else:
             raise TypeError

-    def __radd__(self, other):
+    def __radd__(self, other: str) -> Seq:
         """Add a sequence on the left.

         If adding a string to a Seq, the alphabet is preserved:
@@ -404,7 +408,7 @@ class Seq:
         else:
             raise TypeError

-    def __mul__(self, other):
+    def __mul__(self, other: int) -> Seq:
         """Multiply Seq by integer.

         >>> from Bio.Seq import Seq
@@ -420,7 +424,7 @@ class Seq:
             )
         return self.__class__(str(self) * other, self.alphabet)

-    def __rmul__(self, other):
+    def __rmul__(self, other: int) -> Seq:
         """Multiply integer by Seq.

         >>> from Bio.Seq import Seq
@@ -436,7 +440,7 @@ class Seq:
             )
         return self.__class__(str(self) * other, self.alphabet)

-    def __imul__(self, other):
+    def __imul__(self, other: int) -> Seq:
         """Multiply Seq in-place.

(changes continue)

[peterjc]

Interesting. What script did you run via monkeytype for those inferences? The test suite?

The examples show might warrant defining a custom alias for string, Seq (and subclasses), or MutableSeq?

Does the syntax cope with name-spaced classes, e.g. suppose we were type annotating another file,

• from Bio.Seq import Seq - can use class directly as Seq in type annotation

• from Bio import Seq - can you use Seq.Seq in the annotation?

As you say, this will need careful review and manual work.

[chris-rands]

I actually just used the script itself, but using the test suite is a good idea and improves the inference:

diff --git a/Bio/Seq.py b/Bio/Seq.py
index 71818ec89..be9a68279 100644
--- a/Bio/Seq.py
+++ b/Bio/Seq.py
@@ -35,7 +35,11 @@ from Bio.Data.IUPACData import ambiguous_rna_letters as _ambiguous_rna_letters
 from Bio.Data import CodonTable


-def _maketrans(complement_mapping):
+from Bio.Alphabet import Alphabet, Gapped, HasStopCodon
+from Bio.Data.CodonTable import AmbiguousCodonTable, NCBICodonTableDNA, NCBICodonTableRNA
+from array import array
+from typing import Dict, List, Optional, Tuple, Union
+def _maketrans(complement_mapping: Dict[str, str]) -> Dict[int, int]:
     """Make a python string translation table (PRIVATE).

     Arguments:
@@ -81,7 +85,7 @@ class Seq:
     not applicable to sequences with a protein alphabet).
     """

-    def __init__(self, data, alphabet=Alphabet.generic_alphabet):
+    def __init__(self, data: Union[str, Seq], alphabet: Union[Alphabet, HasStopCodon, Gapped] = Alphabet.generic_alphabet) -> None:
         """Create a Seq object.

         Arguments:
@@ -115,7 +119,7 @@ class Seq:
         self._data = data
         self.alphabet = alphabet  # Seq API requirement

-    def __repr__(self):
+    def __repr__(self) -> str:

(continued)

In fact, if we had complete test suites, I guess the automated inferences would be complete also? So, this process might point to gaps in our test suite?

The only obvious issue to me with using the test suite is I think the exception tests are used to give incorrect inferences, e.g.

-    def __mul__(self, other):
+    def __mul__(self, other: Union[str, int, float]) -> Seq:
         """Multiply Seq by integer.

I'm guessing that these tests are the cause
https://github.com/biopython/biopython/blob/master/Tests/test_seq.py#L559-L565

[chris-rands]

The examples show might warrant defining a custom alias for string, Seq (and subclasses), or MutableSeq?

Maybe an alias yes, it's possible; or maybe if alphabets are removed, Seq could subclass str directly (and MutableSeq could subclass list or an array). Then, many redundant methods that start str(self) could be removed too.

Does the syntax cope with name-spaced classes, e.g. suppose we were type annotating another file,

• from Bio.Seq import Seq - can use class directly as Seq in type annotation
• from Bio import Seq - can you use Seq.Seq in the annotation?

Seq.Seq is a valid annotation, but I don't know if/when monkeytype will insert these

[chris-rands]

I don't know if anyone is working on this (probably not), but I now wonder if it is worth waiting until support for Python 3.6 is dropped before adding variable annotations. I suggest this because Data Classes (new in 3.7) interact rather nicely with variable annotations, making the syntax more light weight and clean. Of course not all classes will become @dataclass decorated, but I suspect some will, e.g.

class Seq:
    """...
    """
    def __init__(self, data: str, alphabet: Any = Alphabet.generic_alphabet) -> None:
        """...
        """
        # Enforce string storage
        if not isinstance(data, str):
            raise TypeError(
                "The sequence data given to a Seq object should "
                "be a string (not another Seq object etc)"
            )
        self._data = data
        self.alphabet = alphabet  # Seq API requirement

becomes...

@dataclass
class Seq:
    """...
    """
    data: str
    alphabet: Any = Alphabet.generic_alphabet

[peterjc]

Roll on Python 3.7+ only, using type annotations like that will be a more interesting transition than the Python 2/3 cleanup we're busy with right now.