Repository: widdowquinn/pyani

Issue:
Convert entrez_retry() to decorator
#### Summary:
The `entrez_retry()` function in downloads could be rewritten as a decorator, applied to each of the entrez download functions

Discussion:

Patch:
diff --git a/.gitignore b/.gitignore
index fb6fe27..1bb86dd 100644
--- a/.gitignore
+++ b/.gitignore
@@ -42,7 +42,9 @@ tests/test_JSpecies/C3/
 tests/test_JSpecies/Corynebacterium/
 tests/test_JSpecies/pyani_3_species/
 C_blochmannia*/
-	
+classes_pyani.pdf
+packages_pyani.pdf
+
 # SGE output
 stderr/
 stdout/
@@ -66,4 +68,8 @@ venv-*
 .pyani
 
 # Ignore mypy config
-.mypy_cache
\ No newline at end of file
+.mypy_cache
+
+# Extra documentation output
+classes_pyani.pdf
+packages_pyani.pdf
\ No newline at end of file
diff --git a/CHANGES.md b/CHANGES.md
index 2b87175..bee5103 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -18,6 +18,10 @@
 - `pyani plot` now produces distribution plots in addition to heatmaps
 - A number of bug fixes were implemented, including:
   - consistent handling of filenames as `Path`s
+  - alignment length calculation for ANIm was corrected
+- major refactoring of code ported from v0.2
+  - static typing implemented
+- testing converted to `pytest` conventions from `unittest`
 
 ## v0.2.7.dev
 
diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md
index b4c134a..ff756b5 100644
--- a/CONTRIBUTORS.md
+++ b/CONTRIBUTORS.md
@@ -5,3 +5,4 @@
 - [ytanizaw](https://github.com/ytanizaw)
 - [Will Trimble](https://github.com/wltrimbl)
 - [Rob Davey](https://github.com/froggleston)
+- [Balázs Brankovics](https://github.com/b-brankovics)
\ No newline at end of file
diff --git a/pyani/anim.py b/pyani/anim.py
index 618e2d0..71f0a37 100644
--- a/pyani/anim.py
+++ b/pyani/anim.py
@@ -235,25 +235,64 @@ def parse_delta(filename: Path) -> Tuple[int, int]:
     aligned uniquely-matched region, and returns the cumulative total for
     each as a tuple.
 
+    Similarity errors are defined in the .delta file spec (see below) as
+    non-positive match scores. For NUCmer output, this is identical to the
+    number of errors (non-identities and indels).
+
     Delta file format has seven numbers in the lines of interest:
+    see http://mummer.sourceforge.net/manual/ for specification
+
     - start on query
     - end on query
     - start on target
     - end on target
     - error count (non-identical, plus indels)
     - similarity errors (non-positive match scores)
+        [NOTE: with PROmer this is equal to error count]
     - stop codons (always zero for nucmer)
+
+    To calculate alignment length, we take the length of the aligned region of
+    the reference (no gaps), and process the delta information. This takes the
+    form of one value per line, following the header sequence. Positive values
+    indicate an insertion in the reference; negative values a deletion in the
+    reference (i.e. an insertion in the query). The total length of the alignment
+    is then:
+
+    reference_length + insertions - deletions
+
+    For example:
+
+    A = ABCDACBDCAC$
+    B = BCCDACDCAC$
+    Delta = (1, -3, 4, 0)
+    A = ABC.DACBDCAC$
+    B = .BCCDAC.DCAC$
+
+    A is the reference and has length 11. There are two insertions (positive delta),
+    and one deletion (negative delta). Alignment length is then 11 + 1 = 12.
     """
-    aln_length, sim_errors = 0, 0
-    for line in [l.strip().split() for l in filename.open("r").readlines()]:
+    in_aln, aln_length, sim_errors = False, 0, 0
+    for line in [_.strip().split() for _ in filename.open("r").readlines()]:
         if line[0] == "NUCMER" or line[0].startswith(">"):  # Skip headers
             continue
-        # We only process lines with seven columns:
+        # Lines with seven columns are alignment region headers:
         if len(line) == 7:
-            aln_length += abs(int(line[1]) - int(line[0]) + 1)
-            indels = int(line[4]) - int(line[5])
-            aln_length -= indels
-            sim_errors += int(line[5])
+            aln_length += abs(int(line[1]) - int(line[0])) + 1  # reference length
+            sim_errors += int(line[4])  # count of non-identities and indels
+            in_aln = True
+        # Lines with a single column (following a header) report numbers of symbols
+        # until next insertion (+ve) or deletion (-ve) in the reference; one line per
+        # insertion/deletion; the alignment always ends with 0
+        if in_aln and line[0].startswith("0"):
+            in_aln = False
+        elif in_aln:
+            # Add one to the alignment length for each reference insertion; subtract
+            # one for each deletion
+            val = int(line[0])
+            if val < 1:  # deletion in reference
+                aln_length += 1
+            elif val == 0:  # ends the alignment entry
+                in_aln = False
     return aln_length, sim_errors
 
 
diff --git a/pyani/download.py b/pyani/download.py
index 9a26b95..9e417ea 100644
--- a/pyani/download.py
+++ b/pyani/download.py
@@ -215,58 +215,57 @@ def download_genome_and_hash(
     return dlstatus  # pylint: disable=no-member
 
 
-# Get results from NCBI web history, in batches
-def entrez_batch_webhistory(
-    record, expected, batchsize, retries, *fnargs, **fnkwargs
-) -> List[str]:
-    """Recover the Entrez data from a prior NCBI webhistory search.
-
-    :param record:  Entrez webhistory record
-    :param expected:  number of expected search returns
-    :param batchsize:  how many search returns to retrieve in a batch
-    :param retries:  int
-    :param *fnargs:  arguments to Efetch
-    :param **fnkwargs:  keyword arguments to Efetch
-
-    Recovers results in batches of defined size, using Efetch.
-    Returns all results as a list.
-    """
-    results = []  # type: List[Any]
-    for start in range(0, expected, batchsize):
-        batch_handle = entrez_retry(
-            Entrez.efetch,
-            retries,
-            retstart=start,
-            retmax=batchsize,
-            webenv=record["WebEnv"],
-            query_key=record["QueryKey"],
-            *fnargs,
-            **fnkwargs,
-        )
-        batch_record = Entrez.read(batch_handle, validate=False)
-        results.extend(batch_record)
-    return results
-
-
-# Retry an Entrez query a specified number of times
-def entrez_retry(func, retries, *fnargs, **fnkwargs):
-    """Retry the passed function up to the number of times specified.
-
-    :param func:  function to be executed
-    :param retries:  int, number of times to retry function execution
-    :param *fnargs:  optional arguments to passed function
-    :param **fnkwargs:  optional keyword arguments to passed function
+def entrez_retry(func):
+    """Decorator to retry the wrapped function up to 'retries' times."""
+
+    def wrapper(*args, retries=1, **kwargs):
+        tries, success = 0, False
+        while not success and tries < retries:
+            try:
+                output = func(*args, **kwargs)
+                success = True
+            except (HTTPError, URLError):
+                tries += 1
+        if not success:
+            raise NCBIDownloadException(f"Too many Entrez failures (limit: {retries})")
+        return Entrez.read(output, validate=False)
+
+    return wrapper
+
+
+def entrez_batch(func):
+    """Decorator to compile batches from the wrapped function into a single set of results.
+
+    The entrez_batch decorator should go outside the entrez_retry decorator.
     """
-    tries, success = 0, False
-    while not success and tries < retries:
-        try:
-            output = func(*fnargs, **fnkwargs)
-            success = True
-        except (HTTPError, URLError):
-            tries += 1
-    if not success:
-        raise NCBIDownloadException("Too many Entrez failures")
-    return output
+
+    def wrapper(*args, expected=None, batchsize=None, **kwargs):
+        if expected is None or batchsize is None:
+            raise ValueError("Expected result count and batchsize must be supplied")
+        results = []  # type: List[Any]
+        for start in range(0, expected, batchsize):
+            # We expect a parsed output
+            output = func(*args, retstart=start, retmax=batchsize, **kwargs)
+            results.extend(output)
+        return results
+
+    return wrapper
+
+
+@entrez_batch
+@entrez_retry
+def entrez_batched_webhistory(*args, **kwargs):
+    return Entrez.efetch(**kwargs)
+
+
+@entrez_retry
+def entrez_esearch(*args, **kwargs):
+    return Entrez.esearch(**kwargs)
+
+
+@entrez_retry
+def entrez_esummary(*args, **kwargs):
+    return Entrez.esummary(*args, **kwargs)
 
 
 # Split a list of taxon ids into components, checking for correct formatting
@@ -299,15 +298,20 @@ def get_asm_uids(taxon_uid: str, retries: int) -> ASMIDs:
 
     # Perform initial search for assembly UIDs with taxon ID as query.
     # Use NCBI history for the search.
-    handle = entrez_retry(
-        Entrez.esearch, retries, db="assembly", term=query, format="xml", usehistory="y"
+    record = entrez_esearch(
+        retries=retries, db="assembly", term=query, format="xml", usehistory="y"
     )
-    record = Entrez.read(handle, validate=False)
     result_count = int(record["Count"])
 
     # Recover assembly UIDs from the web history
-    asm_ids = entrez_batch_webhistory(
-        record, result_count, 250, retries, db="assembly", retmode="xml"
+    asm_ids = entrez_batched_webhistory(
+        expected=result_count,
+        batchsize=250,
+        retries=retries,
+        webenv=record["WebEnv"],
+        query_key=record["QueryKey"],
+        db="assembly",
+        retmode="xml",
     )
 
     return ASMIDs(query, result_count, asm_ids)
@@ -340,16 +344,8 @@ def get_ncbi_esummary(asm_uid, retries, api_key=None) -> Tuple:
     :param api_key:
     """
     # Obtain full eSummary data for the assembly
-    summary = Entrez.read(
-        entrez_retry(
-            Entrez.esummary,
-            retries,
-            db="assembly",
-            id=asm_uid,
-            report="full",
-            api_key=api_key,
-        ),
-        validate=False,
+    summary = entrez_esummary(
+        retries=retries, db="assembly", id=asm_uid, report="full", api_key=api_key
     )
 
     # Extract filestem from assembly data


Test patch:
diff --git a/tests/fixtures/anim/dataframes/deltadir_result.csv b/tests/fixtures/anim/dataframes/deltadir_result.csv
index d7ea29a..51758fd 100644
--- a/tests/fixtures/anim/dataframes/deltadir_result.csv
+++ b/tests/fixtures/anim/dataframes/deltadir_result.csv
@@ -1,5 +1,5 @@
 ,NC_002696,NC_010338,NC_011916,NC_014100
-NC_002696,1.0,0.85112,0.999974,0.86801
-NC_010338,0.85112,1.0,0.8512,0.852965
-NC_011916,0.999974,0.8512,1.0,0.868061
-NC_014100,0.86801,0.852965,0.868061,1.0
+NC_002696,1.000000,0.852072,0.999974,0.868745
+NC_010338,0.852072,1.000000,0.852027,0.853775
+NC_011916,0.999974,0.852027,1.000000,0.868795
+NC_014100,0.868745,0.853775,0.868795,1.000000
diff --git a/tests/test_anim.py b/tests/test_anim.py
index bec077e..d83c552 100644
--- a/tests/test_anim.py
+++ b/tests/test_anim.py
@@ -96,7 +96,7 @@ def delta_output_dir(dir_anim_in):
 @pytest.fixture
 def deltafile_parsed(dir_anim_in):
     """Example parsed deltafile data."""
-    return DeltaParsed(dir_anim_in / "test.delta", (4074001, 2191))
+    return DeltaParsed(dir_anim_in / "test.delta", (4074148, 2191))
 
 
 @pytest.fixture
diff --git a/tests/test_parsing.py b/tests/test_parsing.py
index c82d455..7ec2b42 100644
--- a/tests/test_parsing.py
+++ b/tests/test_parsing.py
@@ -51,4 +51,4 @@ from pyani.anim import parse_delta
 def test_anim_delta(dir_anim_in):
     """Test parsing of NUCmer delta file."""
     aln, sim = parse_delta(dir_anim_in / "NC_002696_vs_NC_011916.delta")
-    assert (aln, sim) == (4074001, 2191)
+    assert (aln, sim) == (4074148, 2191)