Repository: widdowquinn/pyani

Issue:
anim nested tree recovery mode follow up
In testing #351 in order to re-run an analysis from last year (August 2020) with the existing ``nucmer_output/`` folder, and different subsets, a small test case finished fairly quickly. A large subset of about 454 entries (out of the original approx 500) has not. Running ``top`` shows it is calling ``nucmer`` and examination of the ``nucmer_output/`` folder shows new/modified files.

Excerpt from the 17 genome example, note 17*16/2 = 136:

```
...
[INFO] [pyani.scripts.subcommands.subcmd_anim]: Generating ANIm command-lines
[INFO] [pyani.scripts.subcommands.subcmd_anim]: Compiling genomes for comparison
[INFO] [pyani.scripts.subcommands.subcmd_anim]: Compiling pairwise comparisons (this can take time for large datasets)...
100%|██████████| 17/17 [00:00<00:00, 387517.22it/s]
[INFO] [pyani.scripts.subcommands.subcmd_anim]: 	...total parwise comparisons to be performed: 136
[INFO] [pyani.scripts.subcommands.subcmd_anim]: Checking database for existing comparison data...
[INFO] [pyani.scripts.subcommands.subcmd_anim]: 	...after check, still need to run 136 comparisons
[WARNING] [pyani.scripts.subcommands.subcmd_anim]: Entering recovery mode
[INFO] [pyani.scripts.subcommands.subcmd_anim]: Recover mode identified 210692 existing output files for reuse: <snip>.filter (etc)
[INFO] [pyani.scripts.subcommands.subcmd_anim]: Creating NUCmer jobs for ANIm
100%|██████████| 136/136 [00:00<00:00, 526.72it/s]
[INFO] [pyani.scripts.subcommands.subcmd_anim]: Running jobs with multiprocessing
[INFO] [pyani.scripts.subcommands.subcmd_anim]: Multiprocessing run completed without error
[INFO] [pyani.scripts.subcommands.subcmd_anim]: ...jobs complete
[INFO] [pyani.scripts.subcommands.subcmd_anim]: Adding comparison results to database...
100%|██████████| 136/136 [00:05<00:00, 25.95it/s]
[INFO] [pyani.scripts.subcommands.subcmd_anim]: ...database updated.
[INFO] [pyani.scripts.pyani_script]: Completed. Time taken: 22.073
...
```

This does not say how many of cached files are of interest (most are from other genomes not needed here), and how many times ``nucmer`` will be called.

A more elaborate test may be needed to verify no ``nucmer`` calls are made when the cached output should already be complete.

I suspect a glitch in path comparison (e.g. relative vs absolute paths), and the current tests passed doing unnecessary re-computation.

Update: typo

Discussion:

Patch:
diff --git a/pyani/anim.py b/pyani/anim.py
index f338458..de9662a 100644
--- a/pyani/anim.py
+++ b/pyani/anim.py
@@ -233,7 +233,7 @@ def construct_nucmer_cmdline(
     outdir, called "nucmer_output".
     """
     # Cast path strings to pathlib.Path for safety
-    fname1, fname2 = Path(fname1), Path(fname2)
+    fname1, fname2 = sorted([Path(fname1), Path(fname2)])
 
     # Compile commands
     # Nested output folders to avoid N^2 scaling in files-per-folder
diff --git a/pyani/pyani_files.py b/pyani/pyani_files.py
index 022d989..3f10d35 100644
--- a/pyani/pyani_files.py
+++ b/pyani/pyani_files.py
@@ -74,15 +74,15 @@ def get_fasta_paths(
     :param dirname:  Path, path to directory containing input FASTA files
     :param extlist:  List, file suffixes for FASTA files
 
-    Returns the full path to each file.
+    Returns sorted list of the full path to each file.
     """
     # Lists are dangerous to have as default function arguments
     extlist = extlist or [".fna", ".fa", ".fasta", ".fas"]
-    return [
+    return sorted(
         fname
         for fname in dirname.iterdir()
         if fname.is_file() and fname.suffix in extlist
-    ]
+    )
 
 
 # Get a list of FASTA files and corresponding hashes from the input directory
@@ -116,12 +116,12 @@ def get_fasta_and_hash_paths(dirname: Path = Path(".")) -> List[Tuple[Path, Path
 
 # Get list of FASTA files in a directory
 def get_input_files(dirname: Path, *ext) -> List[Path]:
-    """Return files in passed directory, filtered by extension.
+    """Return sorted files in passed directory, filtered by extension.
 
     :param dirname:  Path, path to input directory
     :param *ext:  optional iterable of arguments describing permitted file extensions
     """
-    return [fname for fname in dirname.iterdir() if fname.suffix in ext]
+    return sorted(fname for fname in dirname.iterdir() if fname.suffix in ext)
 
 
 # Get lengths of input sequences
diff --git a/pyani/scripts/subcommands/subcmd_anim.py b/pyani/scripts/subcommands/subcmd_anim.py
index 3cbfaa7..457cb9c 100644
--- a/pyani/scripts/subcommands/subcmd_anim.py
+++ b/pyani/scripts/subcommands/subcmd_anim.py
@@ -320,6 +320,7 @@ def generate_joblist(
     existingfiles = set(existingfiles)  # Path objects hashable
 
     joblist = []  # will hold ComparisonJob structs
+    jobs = {"new": 0, "old": 0}  # will hold counts of new/old jobs for reporting
     for idx, (query, subject) in enumerate(
         tqdm(comparisons, disable=args.disable_tqdm)
     ):
@@ -350,6 +351,7 @@ def generate_joblist(
             logger.debug("Recovering output from %s, not submitting job", outfname)
             # Need to track the expected output, but set the job itself to None:
             joblist.append(ComparisonJob(query, subject, dcmd, ncmd, outfname, None))
+            jobs["old"] += 1
         else:
             logger.debug("Building job")
             # Build jobs
@@ -357,6 +359,14 @@ def generate_joblist(
             fjob = pyani_jobs.Job("%s_%06d-f" % (args.jobprefix, idx), dcmd)
             fjob.add_dependency(njob)
             joblist.append(ComparisonJob(query, subject, dcmd, ncmd, outfname, fjob))
+            jobs["new"] += 1
+    logger.info(
+        "Results not found for %d comparisons; %d new jobs built.",
+        jobs["new"],
+        jobs["new"],
+    )
+    if existingfiles:
+        logger.info("Retrieving results for %d previous comparisons.", jobs["old"])
     return joblist
 
 


Test patch:
diff --git a/tests/conftest.py b/tests/conftest.py
index 61b7c42..e5bb1d5 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -66,6 +66,12 @@ FIXTUREPATH = TESTSPATH / "fixtures"
 
 
 # Convenience structs to emulate returned objects
+class MockGenome(NamedTuple):
+    """Mock genome object."""
+
+    path: str
+
+
 class MockProcess(NamedTuple):
     """Mock process object."""
 
@@ -267,6 +273,13 @@ def fragment_length():
     return FRAGSIZE
 
 
+@pytest.fixture
+def unsorted_genomes(dir_anim_in):
+    """Tests ordering of genome names in output file names for asymmetric analyses."""
+    dir_anim_in = str(dir_anim_in)
+    return (MockGenome(f"{dir_anim_in}/second"), MockGenome(f"{dir_anim_in}/first"))
+
+
 @pytest.fixture
 def mock_legacy_single_genome_dl(monkeypatch):
     """Mocks remote database calls for single-genome downloads.
diff --git a/tests/test_anim.py b/tests/test_anim.py
index ca1c07d..dc3c402 100644
--- a/tests/test_anim.py
+++ b/tests/test_anim.py
@@ -250,3 +250,14 @@ def test_mummer_job_generation(mummer_cmds_four):
         assert job.name == "test_%06d-f" % idx  # filter job name
         assert len(job.dependencies) == 1  # has NUCmer job
         assert job.dependencies[0].name == "test_%06d-n" % idx
+
+
+def test_genome_sorting(tmp_path, unsorted_genomes):
+    second, first = [Path(_.path) for _ in unsorted_genomes]
+    outprefix = f"{tmp_path}/nucmer_output/{first.stem}/{first.stem}_vs_{second.stem}"
+    expected = (
+        f"nucmer --mum -p {outprefix} {first} {second}",
+        f"delta_filter_wrapper.py delta-filter -1 {outprefix}.delta {outprefix}.filter",
+    )
+    nucmercmd, filtercmd = anim.construct_nucmer_cmdline(second, first, tmp_path)
+    assert (nucmercmd, filtercmd) == expected