From 2b0702d953c5799837ce8023dbe50938185f8ee4 Mon Sep 17 00:00:00 2001 From: baileythegreen Date: Fri, 29 Jul 2022 15:11:52 +0100 Subject: [PATCH] Update `get_version()` tests based on discussion surrounding Issue #405 --- pyani/anib.py | 18 +++-- pyani/aniblastall.py | 9 ++- pyani/anim.py | 11 ++- pyani/fastani.py | 12 +++- tests/test_anib.py | 35 ++++++---- tests/test_aniblastall.py | 35 ++++++---- tests/test_anim.py | 35 ++++++---- tests/test_fastani.py | 140 ++++++++++++++------------------------ 8 files changed, 160 insertions(+), 135 deletions(-) diff --git a/pyani/anib.py b/pyani/anib.py index c948bac3..4cabff0a 100644 --- a/pyani/anib.py +++ b/pyani/anib.py @@ -124,14 +124,20 @@ def get_version(blast_exe: Path = pyani_config.BLASTN_DEFAULT) -> str: The following circumstances are explicitly reported as strings + - a value of None given for the executable - no executable at passed path - non-executable file at passed path (this includes cases where the user doesn't have execute permissions on the file) - no version info returned """ try: - blastn_path = Path(shutil.which(blast_exe)) # type:ignore - + # Returns a TypeError if `blast_exe` is None + try: + blastn_path = shutil.which(blast_exe) # type:ignore + except TypeError: + return f"expected path to blastn executable; received {blast_exe}" + # Returns a TypeError if `blastn_path` is not on the PATH + blastn_path = Path(blastn_path) except TypeError: return f"{blast_exe} is not found in $PATH" @@ -418,7 +424,7 @@ def generate_blastn_commands( :param filenames: a list of paths to fragmented input FASTA files :param outdir: path to output directory - :param blastn_exe: path to BLASTN executable + :param blast_exe: path to BLASTN executable :param mode: str, analysis type (ANIb or ANIblastall) Assumes that the fragment sequence input filenames have the form @@ -453,18 +459,18 @@ def construct_blastn_cmdline( fname1: Path, fname2: Path, outdir: Path, - blastn_exe: Path = pyani_config.BLASTN_DEFAULT, + blast_exe: Path = pyani_config.BLASTN_DEFAULT, ) -> str: """Return a single blastn command. :param fname1: :param fname2: :param outdir: - :param blastn_exe: str, path to blastn executable + :param blast_exe: str, path to blastn executable """ prefix = outdir / f"{fname1.stem.replace('-fragments', '')}_vs_{fname2.stem}" return ( - f"{blastn_exe} -out {prefix}.blast_tab -query {fname1} -db {fname2} " + f"{blast_exe} -out {prefix}.blast_tab -query {fname1} -db {fname2} " "-xdrop_gap_final 150 -dust no -evalue 1e-15 -max_target_seqs 1 -outfmt " "'6 qseqid sseqid length mismatch pident nident qlen slen " "qstart qend sstart send positive ppos gaps' " diff --git a/pyani/aniblastall.py b/pyani/aniblastall.py index 32a6025e..9e1c7ce6 100644 --- a/pyani/aniblastall.py +++ b/pyani/aniblastall.py @@ -70,6 +70,7 @@ def get_version(blast_exe: Path = pyani_config.BLASTALL_DEFAULT) -> str: The following circumstances are explicitly reported as strings + - a value of None given for the executable - no executable at passed path - non-executable file at passed path (this includes cases where the user doesn't have execute permissions on the file) - no version info returned @@ -78,7 +79,13 @@ def get_version(blast_exe: Path = pyani_config.BLASTALL_DEFAULT) -> str: logger = logging.getLogger(__name__) try: - blastall_path = Path(shutil.which(blast_exe)) # type:ignore + # Returns a TypeError if `blast_exe` is None + try: + blastall_path = shutil.which(blast_exe) # type:ignore + except TypeError: + return f"expected path to blastall executable; received {blast_exe}" + # Returns a TypeError if `blastall_path` is not on the PATH + blastall_path = Path(blastall_path) except TypeError: return f"{blast_exe} is not found in $PATH" diff --git a/pyani/anim.py b/pyani/anim.py index a638a04d..c9e6f6d0 100644 --- a/pyani/anim.py +++ b/pyani/anim.py @@ -110,14 +110,19 @@ def get_version(nucmer_exe: Path = pyani_config.NUCMER_DEFAULT) -> str: - non-executable file at passed path (this includes cases where the user doesn't have execute permissions on the file) - no version info returned """ - try: - nucmer_path = Path(shutil.which(nucmer_exe)) # type:ignore + # Returns a TypeError if `nucmer_exe` is None + try: + nucmer_path = shutil.which(nucmer_exe) # type:ignore + except TypeError: + return f"expected path to nucmer executable; received {nucmer_exe}" + # Returns a TypeError if `nucmer_path` is not on the PATH + nucmer_path = Path(nucmer_path) except TypeError: return f"{nucmer_exe} is not found in $PATH" if not nucmer_path.is_file(): # no executable - return f"No nucmer at {nucmer_path}" + return f"No nucmer executable at {nucmer_path}" # This should catch cases when the file can't be executed by the user if not os.access(nucmer_path, os.X_OK): # file exists but not executable diff --git a/pyani/fastani.py b/pyani/fastani.py index 4039c9a1..14b601b3 100644 --- a/pyani/fastani.py +++ b/pyani/fastani.py @@ -86,15 +86,25 @@ def get_version(fastani_exe: Path = pyani_config.FASTANI_DEFAULT) -> str: The following circumstances are explicitly reported as strings: + - a value of None given for the executable - no executable at passed path - non-executable file at passed path (this includes cases where the user doesn't have execute permissions on the file) - no version info returned """ try: - fastani_path = Path(shutil.which(fastani_exe)) # type:ignore + # Returns a TypeError if `fastani_exe` is None + try: + fastani_path = shutil.which(fastani_exe) # type:ignore + except TypeError: + return f"expected path to fastANI executable; received {fastani_exe}" + # Returns a TypeError if `fastani_path` is not on the PATH + fastani_path = Path(fastani_path) except TypeError: return f"{fastani_exe} is not found in $PATH" + # If a string that is not an executable is passed to + # shutil.which(), the return value will be None, so + # this check is still needed if fastani_path is None: return f"{fastani_exe} is not found in $PATH" diff --git a/tests/test_anib.py b/tests/test_anib.py index f522a107..c204da04 100644 --- a/tests/test_anib.py +++ b/tests/test_anib.py @@ -130,33 +130,44 @@ def test_get_version_nonetype(): """Test behaviour when no location for the executable is given.""" test_file_0 = None - assert anib.get_version(test_file_0) == f"{test_file_0} is not found in $PATH" + assert ( + anib.get_version(test_file_0) + == f"expected path to blastn executable; received {test_file_0}" + ) + + +# Test case 1: no such file exists +def test_get_version_random_string(): + """Test behaviour when the given 'file' is not one.""" + test_file_1 = "string" + assert anib.get_version(test_file_1) == f"{test_file_1} is not found in $PATH" -# Test case 1: there is no executable + +# Test case 2: there is no executable def test_get_version_no_exe(executable_missing, monkeypatch): """Test behaviour when there is no file at the specified executable location.""" - test_file_1 = Path("/non/existent/blastn") - assert anib.get_version(test_file_1) == f"No blastn executable at {test_file_1}" + test_file_2 = Path("/non/existent/blastn") + assert anib.get_version(test_file_2) == f"No blastn executable at {test_file_2}" -# Test case 2: there is a file, but it is not executable +# Test case 3: there is a file, but it is not executable def test_get_version_exe_not_executable(executable_not_executable, monkeypatch): """Test behaviour when the file at the executable location is not executable.""" - test_file_2 = Path("/non/executable/blastn") + test_file_3 = Path("/non/executable/blastn") assert ( - anib.get_version(test_file_2) - == f"blastn exists at {test_file_2} but not executable" + anib.get_version(test_file_3) + == f"blastn exists at {test_file_3} but not executable" ) -# Test case 3: there is an executable file, but the version can't be retrieved +# Test case 4: there is an executable file, but the version can't be retrieved def test_get_version_exe_no_version(executable_without_version, monkeypatch): """Test behaviour when the version for the executable can not be retrieved.""" - test_file_3 = Path("/missing/version/blastn") + test_file_4 = Path("/missing/version/blastn") assert ( - anib.get_version(test_file_3) - == f"blastn exists at {test_file_3} but could not retrieve version" + anib.get_version(test_file_4) + == f"blastn exists at {test_file_4} but could not retrieve version" ) diff --git a/tests/test_aniblastall.py b/tests/test_aniblastall.py index 4bad4246..b55155eb 100644 --- a/tests/test_aniblastall.py +++ b/tests/test_aniblastall.py @@ -57,34 +57,45 @@ def test_get_version_nonetype(): test_file_0 = None assert ( - aniblastall.get_version(test_file_0) == f"{test_file_0} is not found in $PATH" + aniblastall.get_version(test_file_0) + == f"expected path to blastall executable; received {test_file_0}" ) -# Test case 1: there is no executable +# Test case 1: no such file exists +def test_get_version_random_string(): + """Test behaviour when the given 'file' is not one.""" + test_file_1 = "string" + + assert ( + aniblastall.get_version(test_file_1) == f"{test_file_1} is not found in $PATH" + ) + + +# Test case 2: there is no executable def test_get_version_missing_exe(executable_missing): """Test behaviour when there is no file at the specified executable location.""" - test_file_1 = Path("/non/existent/blastall") - assert aniblastall.get_version(test_file_1) == f"No blastall at {test_file_1}" + test_file_2 = Path("/non/existent/blastall") + assert aniblastall.get_version(test_file_2) == f"No blastall at {test_file_2}" -# Test case 2: there is a file, but it is not executable +# Test case 3: there is a file, but it is not executable def test_get_version_not_executable(executable_not_executable): """Test behaviour when the file at the executable location is not executable.""" - test_file_2 = Path("/non/executable/blastall") + test_file_3 = Path("/non/executable/blastall") assert ( - aniblastall.get_version(test_file_2) - == f"blastall exists at {test_file_2} but not executable" + aniblastall.get_version(test_file_3) + == f"blastall exists at {test_file_3} but not executable" ) -# Test case 3: there is an executable file, but the version can't be retrieved +# Test case 4: there is an executable file, but the version can't be retrieved def test_get_version_no_version(executable_without_version): """Test behaviour when the version for the executable can not be retrieved.""" - test_file_3 = Path("/missing/version/blastall") + test_file_4 = Path("/missing/version/blastall") assert ( - aniblastall.get_version(test_file_3) - == f"blastall exists at {test_file_3} but could not retrieve version" + aniblastall.get_version(test_file_4) + == f"blastall exists at {test_file_4} but could not retrieve version" ) diff --git a/tests/test_anim.py b/tests/test_anim.py index baf5b5b9..4404ba68 100644 --- a/tests/test_anim.py +++ b/tests/test_anim.py @@ -156,33 +156,44 @@ def test_get_version_nonetype(): """Test behaviour when no location for the executable is given.""" test_file_0 = None - assert anim.get_version(test_file_0) == f"{test_file_0} is not found in $PATH" + assert ( + anim.get_version(test_file_0) + == f"expected path to nucmer executable; received {test_file_0}" + ) + + +# Test case 1: no such file exists +def test_get_version_random_string(): + """Test behaviour when the given 'file' is not one.""" + test_file_1 = "string" + assert anim.get_version(test_file_1) == f"{test_file_1} is not found in $PATH" -# Test case 1: there is no executable + +# Test case 2: there is no executable def test_get_version_no_exe(executable_missing): """Test behaviour when there is no file at the specified executable location.""" - test_file_1 = Path("/non/existent/nucmer") - assert anim.get_version(test_file_1) == f"No nucmer at {test_file_1}" + test_file_2 = Path("/non/existent/nucmer") + assert anim.get_version(test_file_2) == f"No nucmer executable at {test_file_2}" -# Test case 2: there is a file, but it is not executable +# Test case 3: there is a file, but it is not executable def test_get_version_exe_not_executable(executable_not_executable): """Test behaviour when the file at the executable location is not executable.""" - test_file_2 = Path("/non/executable/nucmer") + test_file_3 = Path("/non/executable/nucmer") assert ( - anim.get_version(test_file_2) - == f"nucmer exists at {test_file_2} but not executable" + anim.get_version(test_file_3) + == f"nucmer exists at {test_file_3} but not executable" ) -# Test case 3: there is an executable file, but the version can't be retrieved +# Test case 4: there is an executable file, but the version can't be retrieved def test_get_version_exe_no_version(executable_without_version): """Test behaviour when the version for the executable can not be retrieved.""" - test_file_3 = Path("/missing/version/nucmer") + test_file_4 = Path("/missing/version/nucmer") assert ( - anim.get_version(test_file_3) - == f"nucmer exists at {test_file_3} but could not retrieve version" + anim.get_version(test_file_4) + == f"nucmer exists at {test_file_4} but could not retrieve version" ) diff --git a/tests/test_fastani.py b/tests/test_fastani.py index e3bd50d1..81aa0d24 100644 --- a/tests/test_fastani.py +++ b/tests/test_fastani.py @@ -5,16 +5,21 @@ pytest -v """ +import os + from pathlib import Path -from typing import List, NamedTuple +from typing import List, NamedTuple, Tuple +import pandas as pd import pytest import unittest -from pyani import fastani +from pandas.util.testing import assert_frame_equal + +from pyani import fastani, pyani_files, pyani_tools -# Some classes... to be decided +### Some classes... to be decided class ComparisonResult(NamedTuple): @@ -56,70 +61,22 @@ def fastani_cmds_four(path_file_four): # works return FastANIExample( path_file_four, [ - ( - "fastANI -q file1.fna -r file1.fna -o fastani_output/file1_vs_file1.fastani " - "--fragLen 3000 -k 16 --minFraction 0.2" - ), - ( - "fastANI -q file1.fna -r file2.fna -o fastani_output/file1_vs_file2.fastani " - "--fragLen 3000 -k 16 --minFraction 0.2" - ), - ( - "fastANI -q file1.fna -r file3.fna -o fastani_output/file1_vs_file3.fastani " - "--fragLen 3000 -k 16 --minFraction 0.2" - ), - ( - "fastANI -q file1.fna -r file4.fna -o fastani_output/file1_vs_file4.fastani " - "--fragLen 3000 -k 16 --minFraction 0.2" - ), - ( - "fastANI -q file2.fna -r file1.fna -o fastani_output/file2_vs_file1.fastani " - "--fragLen 3000 -k 16 --minFraction 0.2" - ), - ( - "fastANI -q file2.fna -r file2.fna -o fastani_output/file2_vs_file2.fastani " - "--fragLen 3000 -k 16 --minFraction 0.2" - ), - ( - "fastANI -q file2.fna -r file3.fna -o fastani_output/file2_vs_file3.fastani " - "--fragLen 3000 -k 16 --minFraction 0.2" - ), - ( - "fastANI -q file2.fna -r file4.fna -o fastani_output/file2_vs_file4.fastani " - "--fragLen 3000 -k 16 --minFraction 0.2" - ), - ( - "fastANI -q file3.fna -r file1.fna -o fastani_output/file3_vs_file1.fastani " - "--fragLen 3000 -k 16 --minFraction 0.2" - ), - ( - "fastANI -q file3.fna -r file2.fna -o fastani_output/file3_vs_file2.fastani " - "--fragLen 3000 -k 16 --minFraction 0.2" - ), - ( - "fastANI -q file3.fna -r file3.fna -o fastani_output/file3_vs_file3.fastani " - "--fragLen 3000 -k 16 --minFraction 0.2" - ), - ( - "fastANI -q file3.fna -r file4.fna -o fastani_output/file3_vs_file4.fastani " - "--fragLen 3000 -k 16 --minFraction 0.2" - ), - ( - "fastANI -q file4.fna -r file1.fna -o fastani_output/file4_vs_file1.fastani " - "--fragLen 3000 -k 16 --minFraction 0.2" - ), - ( - "fastANI -q file4.fna -r file2.fna -o fastani_output/file4_vs_file2.fastani " - "--fragLen 3000 -k 16 --minFraction 0.2" - ), - ( - "fastANI -q file4.fna -r file3.fna -o fastani_output/file4_vs_file3.fastani " - "--fragLen 3000 -k 16 --minFraction 0.2" - ), - ( - "fastANI -q file4.fna -r file4.fna -o fastani_output/file4_vs_file4.fastani " - "--fragLen 3000 -k 16 --minFraction 0.2" - ), + "fastANI -q file1.fna -r file1.fna -o fastani_output/file1_vs_file1.fastani --fragLen 3000 -k 16 --minFraction 0.2", + "fastANI -q file1.fna -r file2.fna -o fastani_output/file1_vs_file2.fastani --fragLen 3000 -k 16 --minFraction 0.2", + "fastANI -q file1.fna -r file3.fna -o fastani_output/file1_vs_file3.fastani --fragLen 3000 -k 16 --minFraction 0.2", + "fastANI -q file1.fna -r file4.fna -o fastani_output/file1_vs_file4.fastani --fragLen 3000 -k 16 --minFraction 0.2", + "fastANI -q file2.fna -r file1.fna -o fastani_output/file2_vs_file1.fastani --fragLen 3000 -k 16 --minFraction 0.2", + "fastANI -q file2.fna -r file2.fna -o fastani_output/file2_vs_file2.fastani --fragLen 3000 -k 16 --minFraction 0.2", + "fastANI -q file2.fna -r file3.fna -o fastani_output/file2_vs_file3.fastani --fragLen 3000 -k 16 --minFraction 0.2", + "fastANI -q file2.fna -r file4.fna -o fastani_output/file2_vs_file4.fastani --fragLen 3000 -k 16 --minFraction 0.2", + "fastANI -q file3.fna -r file1.fna -o fastani_output/file3_vs_file1.fastani --fragLen 3000 -k 16 --minFraction 0.2", + "fastANI -q file3.fna -r file2.fna -o fastani_output/file3_vs_file2.fastani --fragLen 3000 -k 16 --minFraction 0.2", + "fastANI -q file3.fna -r file3.fna -o fastani_output/file3_vs_file3.fastani --fragLen 3000 -k 16 --minFraction 0.2", + "fastANI -q file3.fna -r file4.fna -o fastani_output/file3_vs_file4.fastani --fragLen 3000 -k 16 --minFraction 0.2", + "fastANI -q file4.fna -r file1.fna -o fastani_output/file4_vs_file1.fastani --fragLen 3000 -k 16 --minFraction 0.2", + "fastANI -q file4.fna -r file2.fna -o fastani_output/file4_vs_file2.fastani --fragLen 3000 -k 16 --minFraction 0.2", + "fastANI -q file4.fna -r file3.fna -o fastani_output/file4_vs_file3.fastani --fragLen 3000 -k 16 --minFraction 0.2", + "fastANI -q file4.fna -r file4.fna -o fastani_output/file4_vs_file4.fastani --fragLen 3000 -k 16 --minFraction 0.2", ], ) @@ -134,37 +91,44 @@ def test_get_version_nonetype(): """Test behaviour when no location for the executable is given.""" test_file_0 = None - assert ( # nosec: B101 - fastani.get_version(test_file_0) == f"{test_file_0} is not found in $PATH" + assert ( + fastani.get_version(test_file_0) + == f"expected path to fastANI executable; received {test_file_0}" ) -# Test case 1: there is no executable +# Test case 1: no such file exists +def test_get_version_random_string(): + """Test behaviour when the given 'file' is not one.""" + test_file_1 = "string" + + assert fastani.get_version(test_file_1) == f"{test_file_1} is not found in $PATH" + + +# Test case 2: there is no executable def test_get_version_no_exe(executable_missing, monkeypatch): """Test behaviour when there is no file at the specified executable location.""" - test_file_1 = Path("/non/existent/blastn") - assert ( # nosec: B101 - fastani.get_version(test_file_1) == f"No fastANI executable at {test_file_1}" - ) + test_file_2 = Path("/non/existent/fastani") + assert fastani.get_version(test_file_2) == f"No fastANI executable at {test_file_2}" -# Test case 2: there is a file, but it is not executable +# Test case 3: there is a file, but it is not executable def test_get_version_exe_not_executable(executable_not_executable, monkeypatch): """Test behaviour when the file at the executable location is not executable.""" - test_file_2 = Path("/non/executable/blastn") - assert ( # nosec: B101 - fastani.get_version(test_file_2) - == f"fastANI exists at {test_file_2} but not executable" # nosec: B101 + test_file_3 = Path("/non/executable/fastani") + assert ( + fastani.get_version(test_file_3) + == f"fastANI exists at {test_file_3} but not executable" ) -# Test case 3: there is an executable file, but the version can't be retrieved +# Test case 4: there is an executable file, but the version can't be retrieved def test_get_version_exe_no_version(executable_without_version, monkeypatch): """Test behaviour when the version for the executable can not be retrieved.""" - test_file_3 = Path("/missing/version/blastn") - assert ( # nosec: B101 - fastani.get_version(test_file_3) - == f"fastANI exists at {test_file_3} but could not retrieve version" + test_file_4 = Path("/missing/version/fastani") + assert ( + fastani.get_version(test_file_4) + == f"fastANI exists at {test_file_4} but could not retrieve version" ) @@ -186,7 +150,7 @@ def test_fastani_single(tmp_path, path_file_two): # works f"-o {dir_fastani / str(path_file_two[0].stem + '_vs_' + path_file_two[1].stem + '.fastani')} " f"--fragLen 3000 -k 16 --minFraction 0.2" ) - assert fastcmd == expected # nosec: B101 + assert fastcmd == expected def test_fastani_multiple(fastani_cmds_four): @@ -197,14 +161,14 @@ def test_fastani_multiple(fastani_cmds_four): cmds = fastani.generate_fastani_commands(fastani_cmds_four.infiles) print(f"\n{cmds}") print((fastani_cmds_four.fastcmds)) - assert cmds == (fastani_cmds_four.fastcmds) # nosec: B101 + assert cmds == (fastani_cmds_four.fastcmds) def test_fastani_job_generation(fastani_cmds_four): # works """Generate job names""" joblist = fastani.generate_fastani_jobs(fastani_cmds_four.infiles, jobprefix="test") - assert len(joblist) == 16 # nosec: B101 + assert len(joblist) == 16 for idx, job in enumerate(joblist): - assert job.name == f"test_{idx:06d}" # nosec: B101 + assert job.name == f"test_{idx:06d}"