From bbcfc213441861f6a4f2f126d2a9565d6fa71779 Mon Sep 17 00:00:00 2001 From: Felicia Sandberg Date: Thu, 30 Oct 2025 15:39:13 +0100 Subject: [PATCH 1/6] Add additional alignment strategy parameters --- q2_alignment/_mafft.py | 55 ++++++++++-- q2_alignment/plugin_setup.py | 82 +++++++++++++++-- q2_alignment/tests/test_mafft.py | 148 +++++++++++++++++++++++++++++++ 3 files changed, 273 insertions(+), 12 deletions(-) diff --git a/q2_alignment/_mafft.py b/q2_alignment/_mafft.py index 0932d91..8d67568 100644 --- a/q2_alignment/_mafft.py +++ b/q2_alignment/_mafft.py @@ -29,7 +29,8 @@ def run_command(cmd, output_fp, verbose=True, env=None): def _mafft(sequences_fp, alignment_fp, n_threads, parttree, addfragments, - keeplength, large): + keeplength, large, globalpair, localpair, genafpair, maxiterate, + retree, nofft, auto): # Save original sequence IDs since long ids (~250 chars) can be truncated # by mafft. We'll replace the IDs in the aligned sequences file output by # mafft with the originals. @@ -107,6 +108,31 @@ def _mafft(sequences_fp, alignment_fp, n_threads, parttree, addfragments, env.update({'MAFFT_TMPDIR': get_cache().get_tmp_path()}) cmd += ['--large'] + if globalpair: + cmd += ['--globalpair'] + + if localpair: + cmd += ['--localpair'] + + if genafpair: + cmd += ['--genafpair'] + + # --maxiterate is set to 0 by default, so we only pass this argument onto + # MAFFT if it deviates from this value. + if maxiterate not in (None, 0): + cmd += ['--maxiterate', str(maxiterate)] + + # --retree is set to 2 by default, so we only pass this argument onto + # MAFFT if it deviates from this value. + if retree not in (None, 2): + cmd += ['--retree', str(retree)] + + if nofft: + cmd += ['--nofft'] + + if auto: + cmd += ['--auto'] + if alignment_fp is not None: add_flag = '--addfragments' if addfragments else '--add' cmd += [add_flag, sequences_fp, alignment_fp] @@ -141,9 +167,19 @@ def _mafft(sequences_fp, alignment_fp, n_threads, parttree, addfragments, def mafft(sequences: DNAFASTAFormat, n_threads: int = 1, parttree: bool = False, - large: bool = False) -> AlignedDNAFASTAFormat: + large: bool = False, + globalpair: bool = False, + localpair: bool = False, + genafpair: bool = False, + maxiterate: int = 0, + retree: int = 2, + nofft: bool = False, + auto: bool = False) -> AlignedDNAFASTAFormat: sequences_fp = str(sequences) - return _mafft(sequences_fp, None, n_threads, parttree, False, False, large) + return _mafft( + sequences_fp, None, n_threads, parttree, False, False, large, + globalpair, localpair, genafpair, maxiterate, retree, nofft, auto + ) def mafft_add(alignment: AlignedDNAFASTAFormat, @@ -152,9 +188,18 @@ def mafft_add(alignment: AlignedDNAFASTAFormat, parttree: bool = False, addfragments: bool = False, keeplength: bool = False, - large: bool = False) -> AlignedDNAFASTAFormat: + large: bool = False, + globalpair: bool = False, + localpair: bool = False, + genafpair: bool = False, + maxiterate: int = 0, + retree: int = 2, + nofft: bool = False, + auto: bool = False) -> AlignedDNAFASTAFormat: alignment_fp = str(alignment) sequences_fp = str(sequences) return _mafft( sequences_fp, alignment_fp, n_threads, parttree, addfragments, - keeplength, large) + keeplength, large, globalpair, localpair, genafpair, maxiterate, + retree, nofft, auto + ) diff --git a/q2_alignment/plugin_setup.py b/q2_alignment/plugin_setup.py index 94e0c06..2ea8dbd 100644 --- a/q2_alignment/plugin_setup.py +++ b/q2_alignment/plugin_setup.py @@ -7,7 +7,7 @@ # ---------------------------------------------------------------------------- from qiime2.plugin import ( - Plugin, Float, Bool, Range, Citations, Threads) + Plugin, Float, Bool, Range, Citations, Threads, Int) from q2_types.feature_data import FeatureData, Sequence, AlignedSequence import q2_alignment @@ -28,19 +28,53 @@ inputs={'sequences': FeatureData[Sequence]}, parameters={'n_threads': Threads, 'parttree': Bool, - 'large': Bool}, + 'large': Bool, + 'globalpair': Bool, + 'localpair': Bool, + 'genafpair': Bool, + 'maxiterate': Int, + 'retree': Int, + 'nofft': Bool, + 'auto': Bool}, outputs=[('alignment', FeatureData[AlignedSequence])], input_descriptions={'sequences': 'The sequences to be aligned.'}, parameter_descriptions={ 'n_threads': 'The number of threads. (Use `auto` to automatically use ' 'all available cores)', 'parttree': 'This flag is required if the number of sequences being ' - 'aligned are larger than 1000000. Disabled by default', + 'aligned are larger than 1000000. Disabled by default.', 'large': 'This flag is required when aligning very large datasets ' 'that do not otherwise fit into memory. Temporary data is ' 'then stored in files, instead of RAM. The --use-cache ' 'flag specifies the storage location of the temporary files ' - 'created. By default, $TMP/qiime2/ is used.'}, + 'created. By default, $TMP/qiime2/ is used.', + 'globalpair': 'Compute all pairwise alignments using the ' + 'Needleman-Wunsch algorithm. Suitable for up to ~200 ' + 'sequences. A combination with --p-maxiterate 1000 ' + 'is recommended (G-INS-i).', + 'localpair': 'Compute all pairwise alignments using the ' + 'Smith-Waterman algorithm. Suitable for up to ~200 ' + 'sequences. A combination with --p-maxiterate 1000 ' + 'is recommended (L-INS-i).', + 'genafpair': 'Compute all pairwise alignments with a local algorithm ' + 'with the generalized affine gap cost. Suitable for up ' + 'to ~200 sequences. A combination with --p-maxiterate ' + '1000 is recommended (E-INS-i).', + 'maxiterate': 'Specifies how many iterative refinement cycles are ' + 'performed after the initial progressive alignment. ' + 'By default, no iterative refinement is performed.', + 'retree': 'Specifies the number of times the guide tree is rebuilt ' + 'during the progressive stage. Typically, tree topology ' + 'stabilizes after 2-3 iterations and higher values rarely ' + 'improves alignment quality enough to justify the extra ' + 'computation.', + 'nofft': 'Disables Fast Fourier Transform (FFT) approximation in ' + 'group-to-group alignment. In general, the FFT algorithm is ' + 'less efficient for distantly related (i.e., less ' + 'conserved) sequences.', + 'auto': 'Automatically select the best alignment strategy ' + '(from FFT-NS-1, FFT-NS-2, FFT-NS-i, or L-INS-i) based on ' + 'the input\'s data size.'}, output_descriptions={'alignment': 'The aligned sequences.'}, name='De novo multiple sequence alignment with MAFFT', description=("Perform de novo multiple sequence alignment using MAFFT."), @@ -55,7 +89,14 @@ 'parttree': Bool, 'addfragments': Bool, 'keeplength': Bool, - 'large': Bool}, + 'large': Bool, + 'globalpair': Bool, + 'localpair': Bool, + 'genafpair': Bool, + 'maxiterate': Int, + 'retree': Int, + 'nofft': Bool, + 'auto': Bool}, outputs=[('expanded_alignment', FeatureData[AlignedSequence])], input_descriptions={'alignment': 'The alignment to which ' 'sequences should be added.', @@ -64,7 +105,7 @@ 'n_threads': 'The number of threads. (Use `auto` to automatically use ' 'all available cores)', 'parttree': 'This flag is required if the number of sequences being ' - 'aligned are larger than 1000000. Disabled by default', + 'aligned are larger than 1000000. Disabled by default.', 'addfragments': 'Optimize for the addition of short sequence ' 'fragments (for example, primer or amplicon ' 'sequences). If not set, default sequence addition ' @@ -78,7 +119,34 @@ 'that do not otherwise fit into memory. Temporary data is ' 'then stored in files, instead of RAM. The --use-cache ' 'flag specifies the storage location of the temporary files ' - 'created. By default, $TMP/qiime2/ is used.'}, + 'created. By default, $TMP/qiime2/ is used.', + 'globalpair': 'Compute all pairwise alignments using the ' + 'Needleman-Wunsch algorithm. Suitable for up to ~200 ' + 'sequences. A combination with --p-maxiterate 1000 ' + 'is recommended (G-INS-i).', + 'localpair': 'Compute all pairwise alignments using the Smith-Waterman' + 'algorithm. Suitable for up to ~200 sequences. A ' + 'combination with --p-maxiterate 1000 is recommended ' + '(L-INS-i).', + 'genafpair': 'Compute all pairwise alignments with a local algorithm ' + 'with the generalized affine gap cost. Suitable for up ' + 'to ~200 sequences. A combination with --p-maxiterate ' + '1000 is recommended (E-INS-i).', + 'maxiterate': 'Specifies how many iterative refinement cycles are ' + 'performed after the initial progressive alignment. ' + 'By default, no iterative refinement is performed.', + 'retree': 'Specifies the number of times the guide tree is rebuilt ' + 'during the progressive stage. Typically, tree topology ' + 'stabilizes after 2-3 iterations and higher values rarely ' + 'improves alignment quality enough to justify the extra ' + 'computation.', + 'nofft': 'Disables Fast Fourier Transform (FFT) approximation in ' + 'group-to-group alignment. In general, the FFT algorithm is ' + 'less efficient for distantly related (i.e., less ' + 'conserved) sequences.', + 'auto': 'Automatically select the best alignment strategy ' + '(from FFT-NS-1, FFT-NS-2, FFT-NS-i, or L-INS-i) based on ' + 'the input\'s data size.'}, output_descriptions={ 'expanded_alignment': 'Alignment containing the provided aligned and ' 'unaligned sequences.'}, diff --git a/q2_alignment/tests/test_mafft.py b/q2_alignment/tests/test_mafft.py index 574d812..3929cae 100644 --- a/q2_alignment/tests/test_mafft.py +++ b/q2_alignment/tests/test_mafft.py @@ -82,6 +82,21 @@ def test_mafft_parttree_exception(self): with redirected_stdio(stderr=os.devnull): mafft(input_sequences) + @patch('q2_alignment._mafft.skbio.TabularMSA.read') + @patch('q2_alignment._mafft.run_command') + def test_mafft_parttree_flag(self, mock_run_cmd, mock_read): + input_sequences, exp = self._prepare_sequence_data() + mock_read.return_value = exp + + mafft(input_sequences, parttree=True) + + mock_run_cmd.assert_called_with( + ["mafft", "--preservecase", "--inputorder", + "--thread", "1", "--parttree", ANY], + ANY, + env=None + ) + def test_mafft_large(self): input_sequences, exp = self._prepare_sequence_data() @@ -91,6 +106,139 @@ def test_mafft_large(self): constructor=skbio.DNA) self.assertEqual(obs, exp) + @patch('q2_alignment._mafft.skbio.TabularMSA.read') + @patch('q2_alignment._mafft.run_command') + def test_mafft_globalpair_flag(self, mock_run_cmd, mock_read): + input_sequences, exp = self._prepare_sequence_data() + mock_read.return_value = exp + + mafft(input_sequences, globalpair=True) + + mock_run_cmd.assert_called_with( + ["mafft", "--preservecase", "--inputorder", + "--thread", "1", "--globalpair", ANY], + ANY, + env=None + ) + + @patch('q2_alignment._mafft.skbio.TabularMSA.read') + @patch('q2_alignment._mafft.run_command') + def test_mafft_localpair_flag(self, mock_run_cmd, mock_read): + input_sequences, exp = self._prepare_sequence_data() + mock_read.return_value = exp + + mafft(input_sequences, localpair=True) + + mock_run_cmd.assert_called_with( + ["mafft", "--preservecase", "--inputorder", + "--thread", "1", "--localpair", ANY], + ANY, + env=None + ) + + @patch('q2_alignment._mafft.skbio.TabularMSA.read') + @patch('q2_alignment._mafft.run_command') + def test_mafft_genafpair_flag(self, mock_run_cmd, mock_read): + input_sequences, exp = self._prepare_sequence_data() + mock_read.return_value = exp + + mafft(input_sequences, genafpair=True) + + mock_run_cmd.assert_called_with( + ["mafft", "--preservecase", "--inputorder", + "--thread", "1", "--genafpair", ANY], + ANY, + env=None + ) + + @patch('q2_alignment._mafft.skbio.TabularMSA.read') + @patch('q2_alignment._mafft.run_command') + def test_mafft_maxiterate_flag(self, mock_run_cmd, mock_read): + input_sequences, exp = self._prepare_sequence_data() + mock_read.return_value = exp + + mafft(input_sequences, maxiterate=1000) + + mock_run_cmd.assert_called_with( + ["mafft", "--preservecase", "--inputorder", + "--thread", "1", "--maxiterate", "1000", ANY], + ANY, + env=None + ) + + @patch('q2_alignment._mafft.skbio.TabularMSA.read') + @patch('q2_alignment._mafft.run_command') + def test_mafft_maxiterate_not_included_when_default( + self, mock_run_cmd, mock_read): + input_sequences, exp = self._prepare_sequence_data() + mock_read.return_value = exp + + mafft(input_sequences, maxiterate=0) + + call_args, _ = mock_run_cmd.call_args + cmd = call_args[0] + + self.assertNotIn("--maxiterate", cmd) + + @patch('q2_alignment._mafft.skbio.TabularMSA.read') + @patch('q2_alignment._mafft.run_command') + def test_mafft_retree_flag(self, mock_run_cmd, mock_read): + input_sequences, exp = self._prepare_sequence_data() + mock_read.return_value = exp + + mafft(input_sequences, retree=3) + + mock_run_cmd.assert_called_with( + ["mafft", "--preservecase", "--inputorder", + "--thread", "1", "--retree", "3", ANY], + ANY, + env=None + ) + + @patch('q2_alignment._mafft.skbio.TabularMSA.read') + @patch('q2_alignment._mafft.run_command') + def test_mafft_retree_not_included_when_default( + self, mock_run_cmd, mock_read): + input_sequences, exp = self._prepare_sequence_data() + mock_read.return_value = exp + + mafft(input_sequences, retree=2) + + call_args, _ = mock_run_cmd.call_args + cmd = call_args[0] + + self.assertNotIn("--retree", cmd) + + @patch('q2_alignment._mafft.skbio.TabularMSA.read') + @patch('q2_alignment._mafft.run_command') + def test_mafft_nofft_flag(self, mock_run_cmd, mock_read): + input_sequences, exp = self._prepare_sequence_data() + mock_read.return_value = exp + + mafft(input_sequences, nofft=True) + + mock_run_cmd.assert_called_with( + ["mafft", "--preservecase", "--inputorder", + "--thread", "1", "--nofft", ANY], + ANY, + env=None + ) + + @patch('q2_alignment._mafft.skbio.TabularMSA.read') + @patch('q2_alignment._mafft.run_command') + def test_mafft_auto_flag(self, mock_run_cmd, mock_read): + input_sequences, exp = self._prepare_sequence_data() + mock_read.return_value = exp + + mafft(input_sequences, auto=True) + + mock_run_cmd.assert_called_with( + ["mafft", "--preservecase", "--inputorder", + "--thread", "1", "--auto", ANY], + ANY, + env=None + ) + class MafftAddTests(TestPluginBase): package = 'q2_alignment.tests' From 6ed5d572a1459567d3af6db04d36ccbd6061f553 Mon Sep 17 00:00:00 2001 From: Felicia Sandberg Date: Mon, 12 Jan 2026 12:06:00 +0100 Subject: [PATCH 2/6] Move alignment strategy flags to single flag, strategy --- q2_alignment/_mafft.py | 69 ++++++++------- q2_alignment/plugin_setup.py | 141 ++++++++++--------------------- q2_alignment/tests/test_mafft.py | 55 +++++++++--- 3 files changed, 127 insertions(+), 138 deletions(-) diff --git a/q2_alignment/_mafft.py b/q2_alignment/_mafft.py index 8d67568..fa92a7f 100644 --- a/q2_alignment/_mafft.py +++ b/q2_alignment/_mafft.py @@ -15,6 +15,34 @@ from qiime2 import get_cache +def _validate_alignment_strategy(strategy: str | None) -> str: + valid_strategies = [ + "auto", "fftns", "nofft", "globalpair", "localpair", "genafpair", + ] + valid_strategies_str = ", ".join( + [s for s in valid_strategies]) + + if strategy == "auto": + return ["--auto"] + if strategy == "fftns": + return [] + if strategy == "nofft": + return ["--nofft"] + if strategy == "localpair": + return ["--localpair"] + if strategy == "globalpair": + return ["--globalpair"] + if strategy == "genafpair": + return ["--genafpair"] + if strategy is None: + return [] + else: + raise ValueError( + f"Invalid alignment strategy '{strategy}'. " + f"Valid values are: {valid_strategies_str}." + ) + + def run_command(cmd, output_fp, verbose=True, env=None): if verbose: print("Running external command line application. This may print " @@ -29,8 +57,7 @@ def run_command(cmd, output_fp, verbose=True, env=None): def _mafft(sequences_fp, alignment_fp, n_threads, parttree, addfragments, - keeplength, large, globalpair, localpair, genafpair, maxiterate, - retree, nofft, auto): + keeplength, large, strategy, maxiterate, retree): # Save original sequence IDs since long ids (~250 chars) can be truncated # by mafft. We'll replace the IDs in the aligned sequences file output by # mafft with the originals. @@ -108,14 +135,9 @@ def _mafft(sequences_fp, alignment_fp, n_threads, parttree, addfragments, env.update({'MAFFT_TMPDIR': get_cache().get_tmp_path()}) cmd += ['--large'] - if globalpair: - cmd += ['--globalpair'] - - if localpair: - cmd += ['--localpair'] - - if genafpair: - cmd += ['--genafpair'] + if strategy: + strategy_flag = _validate_alignment_strategy(strategy) + cmd += strategy_flag # --maxiterate is set to 0 by default, so we only pass this argument onto # MAFFT if it deviates from this value. @@ -127,12 +149,6 @@ def _mafft(sequences_fp, alignment_fp, n_threads, parttree, addfragments, if retree not in (None, 2): cmd += ['--retree', str(retree)] - if nofft: - cmd += ['--nofft'] - - if auto: - cmd += ['--auto'] - if alignment_fp is not None: add_flag = '--addfragments' if addfragments else '--add' cmd += [add_flag, sequences_fp, alignment_fp] @@ -168,17 +184,13 @@ def mafft(sequences: DNAFASTAFormat, n_threads: int = 1, parttree: bool = False, large: bool = False, - globalpair: bool = False, - localpair: bool = False, - genafpair: bool = False, + strategy: str = "fftns", maxiterate: int = 0, - retree: int = 2, - nofft: bool = False, - auto: bool = False) -> AlignedDNAFASTAFormat: + retree: int = 2,) -> AlignedDNAFASTAFormat: sequences_fp = str(sequences) return _mafft( sequences_fp, None, n_threads, parttree, False, False, large, - globalpair, localpair, genafpair, maxiterate, retree, nofft, auto + strategy, maxiterate, retree ) @@ -189,17 +201,12 @@ def mafft_add(alignment: AlignedDNAFASTAFormat, addfragments: bool = False, keeplength: bool = False, large: bool = False, - globalpair: bool = False, - localpair: bool = False, - genafpair: bool = False, + strategy: str = "fftns", maxiterate: int = 0, - retree: int = 2, - nofft: bool = False, - auto: bool = False) -> AlignedDNAFASTAFormat: + retree: int = 2,) -> AlignedDNAFASTAFormat: alignment_fp = str(alignment) sequences_fp = str(sequences) return _mafft( sequences_fp, alignment_fp, n_threads, parttree, addfragments, - keeplength, large, globalpair, localpair, genafpair, maxiterate, - retree, nofft, auto + keeplength, large, strategy, maxiterate, retree ) diff --git a/q2_alignment/plugin_setup.py b/q2_alignment/plugin_setup.py index 2ea8dbd..b6a6334 100644 --- a/q2_alignment/plugin_setup.py +++ b/q2_alignment/plugin_setup.py @@ -7,11 +7,43 @@ # ---------------------------------------------------------------------------- from qiime2.plugin import ( - Plugin, Float, Bool, Range, Citations, Threads, Int) + Plugin, Float, Bool, Range, Citations, Threads, Int, Str) from q2_types.feature_data import FeatureData, Sequence, AlignedSequence import q2_alignment +mafft_params = { + "n_threads": Threads, + "parttree": Bool, + "large": Bool, + "strategy": Str, + "maxiterate": Int, + "retree": Int, +} +mafft_param_descriptions = { + "n_threads": "The number of threads. (Use `auto` to automatically use " + "all available cores)", + "parttree": "This flag is required if the number of sequences being " + "aligned are larger than 1,000,000. Disabled by default.", + "large": "This flag is required when aligning very large datasets " + "that do not otherwise fit into memory. Temporary data is " + "then stored in files, instead of RAM. The --use-cache " + "flag specifies the storage location of the temporary files " + "created. By default, $TMP/qiime2/ is used.", + "strategy": "Specifies the multiple alignment strategy to use. " + "Exactly one strategy may be specified. Valid options " + "are: 'auto', 'fftns', 'nofft', 'globalpair', " + "'localpair', and 'genafpair'. Default strategy: 'fftns'.", + 'maxiterate': 'Specifies how many iterative refinement cycles are ' + 'performed after the initial progressive alignment. ' + 'By default, no iterative refinement is performed.', + 'retree': 'Specifies the number of times the guide tree is rebuilt ' + 'during the progressive stage. Typically, tree topology ' + 'stabilizes after 2-3 iterations and higher values rarely ' + 'improves alignment quality enough to justify the extra ' + 'computation.', +} + citations = Citations.load('citations.bib', package='q2_alignment') plugin = Plugin( name='alignment', @@ -26,55 +58,14 @@ plugin.methods.register_function( function=q2_alignment.mafft, inputs={'sequences': FeatureData[Sequence]}, - parameters={'n_threads': Threads, - 'parttree': Bool, - 'large': Bool, - 'globalpair': Bool, - 'localpair': Bool, - 'genafpair': Bool, - 'maxiterate': Int, - 'retree': Int, - 'nofft': Bool, - 'auto': Bool}, + parameters={ + **mafft_params, + }, outputs=[('alignment', FeatureData[AlignedSequence])], input_descriptions={'sequences': 'The sequences to be aligned.'}, parameter_descriptions={ - 'n_threads': 'The number of threads. (Use `auto` to automatically use ' - 'all available cores)', - 'parttree': 'This flag is required if the number of sequences being ' - 'aligned are larger than 1000000. Disabled by default.', - 'large': 'This flag is required when aligning very large datasets ' - 'that do not otherwise fit into memory. Temporary data is ' - 'then stored in files, instead of RAM. The --use-cache ' - 'flag specifies the storage location of the temporary files ' - 'created. By default, $TMP/qiime2/ is used.', - 'globalpair': 'Compute all pairwise alignments using the ' - 'Needleman-Wunsch algorithm. Suitable for up to ~200 ' - 'sequences. A combination with --p-maxiterate 1000 ' - 'is recommended (G-INS-i).', - 'localpair': 'Compute all pairwise alignments using the ' - 'Smith-Waterman algorithm. Suitable for up to ~200 ' - 'sequences. A combination with --p-maxiterate 1000 ' - 'is recommended (L-INS-i).', - 'genafpair': 'Compute all pairwise alignments with a local algorithm ' - 'with the generalized affine gap cost. Suitable for up ' - 'to ~200 sequences. A combination with --p-maxiterate ' - '1000 is recommended (E-INS-i).', - 'maxiterate': 'Specifies how many iterative refinement cycles are ' - 'performed after the initial progressive alignment. ' - 'By default, no iterative refinement is performed.', - 'retree': 'Specifies the number of times the guide tree is rebuilt ' - 'during the progressive stage. Typically, tree topology ' - 'stabilizes after 2-3 iterations and higher values rarely ' - 'improves alignment quality enough to justify the extra ' - 'computation.', - 'nofft': 'Disables Fast Fourier Transform (FFT) approximation in ' - 'group-to-group alignment. In general, the FFT algorithm is ' - 'less efficient for distantly related (i.e., less ' - 'conserved) sequences.', - 'auto': 'Automatically select the best alignment strategy ' - '(from FFT-NS-1, FFT-NS-2, FFT-NS-i, or L-INS-i) based on ' - 'the input\'s data size.'}, + **mafft_param_descriptions, + }, output_descriptions={'alignment': 'The aligned sequences.'}, name='De novo multiple sequence alignment with MAFFT', description=("Perform de novo multiple sequence alignment using MAFFT."), @@ -85,27 +76,17 @@ function=q2_alignment.mafft_add, inputs={'alignment': FeatureData[AlignedSequence], 'sequences': FeatureData[Sequence]}, - parameters={'n_threads': Threads, - 'parttree': Bool, - 'addfragments': Bool, - 'keeplength': Bool, - 'large': Bool, - 'globalpair': Bool, - 'localpair': Bool, - 'genafpair': Bool, - 'maxiterate': Int, - 'retree': Int, - 'nofft': Bool, - 'auto': Bool}, + parameters={ + **mafft_params, + 'addfragments': Bool, + 'keeplength': Bool, + }, outputs=[('expanded_alignment', FeatureData[AlignedSequence])], input_descriptions={'alignment': 'The alignment to which ' 'sequences should be added.', 'sequences': 'The sequences to be added.'}, parameter_descriptions={ - 'n_threads': 'The number of threads. (Use `auto` to automatically use ' - 'all available cores)', - 'parttree': 'This flag is required if the number of sequences being ' - 'aligned are larger than 1000000. Disabled by default.', + **mafft_param_descriptions, 'addfragments': 'Optimize for the addition of short sequence ' 'fragments (for example, primer or amplicon ' 'sequences). If not set, default sequence addition ' @@ -114,39 +95,7 @@ 'Any added sequence that would otherwise introduce new ' 'insertions into the alignment, will have those ' 'insertions deleted, to preserve original alignment ' - 'length.', - 'large': 'This flag is required when aligning very large datasets ' - 'that do not otherwise fit into memory. Temporary data is ' - 'then stored in files, instead of RAM. The --use-cache ' - 'flag specifies the storage location of the temporary files ' - 'created. By default, $TMP/qiime2/ is used.', - 'globalpair': 'Compute all pairwise alignments using the ' - 'Needleman-Wunsch algorithm. Suitable for up to ~200 ' - 'sequences. A combination with --p-maxiterate 1000 ' - 'is recommended (G-INS-i).', - 'localpair': 'Compute all pairwise alignments using the Smith-Waterman' - 'algorithm. Suitable for up to ~200 sequences. A ' - 'combination with --p-maxiterate 1000 is recommended ' - '(L-INS-i).', - 'genafpair': 'Compute all pairwise alignments with a local algorithm ' - 'with the generalized affine gap cost. Suitable for up ' - 'to ~200 sequences. A combination with --p-maxiterate ' - '1000 is recommended (E-INS-i).', - 'maxiterate': 'Specifies how many iterative refinement cycles are ' - 'performed after the initial progressive alignment. ' - 'By default, no iterative refinement is performed.', - 'retree': 'Specifies the number of times the guide tree is rebuilt ' - 'during the progressive stage. Typically, tree topology ' - 'stabilizes after 2-3 iterations and higher values rarely ' - 'improves alignment quality enough to justify the extra ' - 'computation.', - 'nofft': 'Disables Fast Fourier Transform (FFT) approximation in ' - 'group-to-group alignment. In general, the FFT algorithm is ' - 'less efficient for distantly related (i.e., less ' - 'conserved) sequences.', - 'auto': 'Automatically select the best alignment strategy ' - '(from FFT-NS-1, FFT-NS-2, FFT-NS-i, or L-INS-i) based on ' - 'the input\'s data size.'}, + 'length.'}, output_descriptions={ 'expanded_alignment': 'Alignment containing the provided aligned and ' 'unaligned sequences.'}, diff --git a/q2_alignment/tests/test_mafft.py b/q2_alignment/tests/test_mafft.py index 3929cae..719510b 100644 --- a/q2_alignment/tests/test_mafft.py +++ b/q2_alignment/tests/test_mafft.py @@ -16,7 +16,7 @@ from qiime2.util import redirected_stdio from q2_alignment import mafft, mafft_add -from q2_alignment._mafft import run_command +from q2_alignment._mafft import run_command, _validate_alignment_strategy class MafftTests(TestPluginBase): @@ -106,13 +106,31 @@ def test_mafft_large(self): constructor=skbio.DNA) self.assertEqual(obs, exp) + def test_validate_alignment_strategy_valid(self): + cases = [ + ("auto", ["--auto"]), + ("fftns", []), + ("nofft", ["--nofft"]), + ("localpair", ["--localpair"]), + ("globalpair", ["--globalpair"]), + ("genafpair", ["--genafpair"]), + (None, []), + ] + + for strategy, expected in cases: + assert _validate_alignment_strategy(strategy) == expected + + def test_validate_alignment_strategy_invalid(self): + with self.assertRaisesRegex(ValueError, 'Invalid alignment strategy'): + _validate_alignment_strategy("invalid") + @patch('q2_alignment._mafft.skbio.TabularMSA.read') @patch('q2_alignment._mafft.run_command') - def test_mafft_globalpair_flag(self, mock_run_cmd, mock_read): + def test_mafft_globalpair_strategy(self, mock_run_cmd, mock_read): input_sequences, exp = self._prepare_sequence_data() mock_read.return_value = exp - mafft(input_sequences, globalpair=True) + mafft(input_sequences, strategy="globalpair") mock_run_cmd.assert_called_with( ["mafft", "--preservecase", "--inputorder", @@ -123,11 +141,11 @@ def test_mafft_globalpair_flag(self, mock_run_cmd, mock_read): @patch('q2_alignment._mafft.skbio.TabularMSA.read') @patch('q2_alignment._mafft.run_command') - def test_mafft_localpair_flag(self, mock_run_cmd, mock_read): + def test_mafft_localpair_strategy(self, mock_run_cmd, mock_read): input_sequences, exp = self._prepare_sequence_data() mock_read.return_value = exp - mafft(input_sequences, localpair=True) + mafft(input_sequences, strategy="localpair") mock_run_cmd.assert_called_with( ["mafft", "--preservecase", "--inputorder", @@ -138,11 +156,11 @@ def test_mafft_localpair_flag(self, mock_run_cmd, mock_read): @patch('q2_alignment._mafft.skbio.TabularMSA.read') @patch('q2_alignment._mafft.run_command') - def test_mafft_genafpair_flag(self, mock_run_cmd, mock_read): + def test_mafft_genafpair_strategy(self, mock_run_cmd, mock_read): input_sequences, exp = self._prepare_sequence_data() mock_read.return_value = exp - mafft(input_sequences, genafpair=True) + mafft(input_sequences, strategy="genafpair") mock_run_cmd.assert_called_with( ["mafft", "--preservecase", "--inputorder", @@ -211,11 +229,11 @@ def test_mafft_retree_not_included_when_default( @patch('q2_alignment._mafft.skbio.TabularMSA.read') @patch('q2_alignment._mafft.run_command') - def test_mafft_nofft_flag(self, mock_run_cmd, mock_read): + def test_mafft_nofft_strategy(self, mock_run_cmd, mock_read): input_sequences, exp = self._prepare_sequence_data() mock_read.return_value = exp - mafft(input_sequences, nofft=True) + mafft(input_sequences, strategy="nofft") mock_run_cmd.assert_called_with( ["mafft", "--preservecase", "--inputorder", @@ -226,11 +244,11 @@ def test_mafft_nofft_flag(self, mock_run_cmd, mock_read): @patch('q2_alignment._mafft.skbio.TabularMSA.read') @patch('q2_alignment._mafft.run_command') - def test_mafft_auto_flag(self, mock_run_cmd, mock_read): + def test_mafft_auto_strategy(self, mock_run_cmd, mock_read): input_sequences, exp = self._prepare_sequence_data() mock_read.return_value = exp - mafft(input_sequences, auto=True) + mafft(input_sequences, strategy="auto") mock_run_cmd.assert_called_with( ["mafft", "--preservecase", "--inputorder", @@ -239,6 +257,21 @@ def test_mafft_auto_flag(self, mock_run_cmd, mock_read): env=None ) + @patch('q2_alignment._mafft.skbio.TabularMSA.read') + @patch('q2_alignment._mafft.run_command') + def test_mafft_fftns_strategy(self, mock_run_cmd, mock_read): + input_sequences, exp = self._prepare_sequence_data() + mock_read.return_value = exp + + mafft(input_sequences, strategy="fftns") + + mock_run_cmd.assert_called_with( + ["mafft", "--preservecase", "--inputorder", + "--thread", "1", ANY], + ANY, + env=None + ) + class MafftAddTests(TestPluginBase): package = 'q2_alignment.tests' From eb83728cfd98dcf260b87a3aecdc51264742dd57 Mon Sep 17 00:00:00 2001 From: Felicia Sandberg Date: Tue, 13 Jan 2026 14:01:25 +0100 Subject: [PATCH 3/6] Simplify strategy param parsing --- q2_alignment/_mafft.py | 35 +++----------------------------- q2_alignment/plugin_setup.py | 6 ++++-- q2_alignment/tests/test_mafft.py | 35 +------------------------------- 3 files changed, 8 insertions(+), 68 deletions(-) diff --git a/q2_alignment/_mafft.py b/q2_alignment/_mafft.py index fa92a7f..9ddf28a 100644 --- a/q2_alignment/_mafft.py +++ b/q2_alignment/_mafft.py @@ -15,34 +15,6 @@ from qiime2 import get_cache -def _validate_alignment_strategy(strategy: str | None) -> str: - valid_strategies = [ - "auto", "fftns", "nofft", "globalpair", "localpair", "genafpair", - ] - valid_strategies_str = ", ".join( - [s for s in valid_strategies]) - - if strategy == "auto": - return ["--auto"] - if strategy == "fftns": - return [] - if strategy == "nofft": - return ["--nofft"] - if strategy == "localpair": - return ["--localpair"] - if strategy == "globalpair": - return ["--globalpair"] - if strategy == "genafpair": - return ["--genafpair"] - if strategy is None: - return [] - else: - raise ValueError( - f"Invalid alignment strategy '{strategy}'. " - f"Valid values are: {valid_strategies_str}." - ) - - def run_command(cmd, output_fp, verbose=True, env=None): if verbose: print("Running external command line application. This may print " @@ -136,8 +108,7 @@ def _mafft(sequences_fp, alignment_fp, n_threads, parttree, addfragments, cmd += ['--large'] if strategy: - strategy_flag = _validate_alignment_strategy(strategy) - cmd += strategy_flag + cmd += [("--" + strategy)] # --maxiterate is set to 0 by default, so we only pass this argument onto # MAFFT if it deviates from this value. @@ -184,7 +155,7 @@ def mafft(sequences: DNAFASTAFormat, n_threads: int = 1, parttree: bool = False, large: bool = False, - strategy: str = "fftns", + strategy: str | None = None, maxiterate: int = 0, retree: int = 2,) -> AlignedDNAFASTAFormat: sequences_fp = str(sequences) @@ -201,7 +172,7 @@ def mafft_add(alignment: AlignedDNAFASTAFormat, addfragments: bool = False, keeplength: bool = False, large: bool = False, - strategy: str = "fftns", + strategy: str | None = None, maxiterate: int = 0, retree: int = 2,) -> AlignedDNAFASTAFormat: alignment_fp = str(alignment) diff --git a/q2_alignment/plugin_setup.py b/q2_alignment/plugin_setup.py index b6a6334..84646d3 100644 --- a/q2_alignment/plugin_setup.py +++ b/q2_alignment/plugin_setup.py @@ -7,7 +7,7 @@ # ---------------------------------------------------------------------------- from qiime2.plugin import ( - Plugin, Float, Bool, Range, Citations, Threads, Int, Str) + Plugin, Float, Bool, Range, Citations, Threads, Int, Str, Choices) from q2_types.feature_data import FeatureData, Sequence, AlignedSequence import q2_alignment @@ -16,7 +16,9 @@ "n_threads": Threads, "parttree": Bool, "large": Bool, - "strategy": Str, + "strategy": Str % Choices({ + "auto", "nofft", "globalpair", "localpair", "genafpair", + }), "maxiterate": Int, "retree": Int, } diff --git a/q2_alignment/tests/test_mafft.py b/q2_alignment/tests/test_mafft.py index 719510b..10248c1 100644 --- a/q2_alignment/tests/test_mafft.py +++ b/q2_alignment/tests/test_mafft.py @@ -16,7 +16,7 @@ from qiime2.util import redirected_stdio from q2_alignment import mafft, mafft_add -from q2_alignment._mafft import run_command, _validate_alignment_strategy +from q2_alignment._mafft import run_command class MafftTests(TestPluginBase): @@ -106,24 +106,6 @@ def test_mafft_large(self): constructor=skbio.DNA) self.assertEqual(obs, exp) - def test_validate_alignment_strategy_valid(self): - cases = [ - ("auto", ["--auto"]), - ("fftns", []), - ("nofft", ["--nofft"]), - ("localpair", ["--localpair"]), - ("globalpair", ["--globalpair"]), - ("genafpair", ["--genafpair"]), - (None, []), - ] - - for strategy, expected in cases: - assert _validate_alignment_strategy(strategy) == expected - - def test_validate_alignment_strategy_invalid(self): - with self.assertRaisesRegex(ValueError, 'Invalid alignment strategy'): - _validate_alignment_strategy("invalid") - @patch('q2_alignment._mafft.skbio.TabularMSA.read') @patch('q2_alignment._mafft.run_command') def test_mafft_globalpair_strategy(self, mock_run_cmd, mock_read): @@ -257,21 +239,6 @@ def test_mafft_auto_strategy(self, mock_run_cmd, mock_read): env=None ) - @patch('q2_alignment._mafft.skbio.TabularMSA.read') - @patch('q2_alignment._mafft.run_command') - def test_mafft_fftns_strategy(self, mock_run_cmd, mock_read): - input_sequences, exp = self._prepare_sequence_data() - mock_read.return_value = exp - - mafft(input_sequences, strategy="fftns") - - mock_run_cmd.assert_called_with( - ["mafft", "--preservecase", "--inputorder", - "--thread", "1", ANY], - ANY, - env=None - ) - class MafftAddTests(TestPluginBase): package = 'q2_alignment.tests' From 38fb3a8b719c0ee6542334be7efb89b8071400b9 Mon Sep 17 00:00:00 2001 From: Felicia Sandberg Date: Tue, 13 Jan 2026 14:41:01 +0100 Subject: [PATCH 4/6] Update param descriptions to reflect changes --- q2_alignment/plugin_setup.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/q2_alignment/plugin_setup.py b/q2_alignment/plugin_setup.py index 84646d3..59c4c4c 100644 --- a/q2_alignment/plugin_setup.py +++ b/q2_alignment/plugin_setup.py @@ -34,8 +34,8 @@ "created. By default, $TMP/qiime2/ is used.", "strategy": "Specifies the multiple alignment strategy to use. " "Exactly one strategy may be specified. Valid options " - "are: 'auto', 'fftns', 'nofft', 'globalpair', " - "'localpair', and 'genafpair'. Default strategy: 'fftns'.", + "are: 'auto', 'nofft', 'globalpair', 'localpair', " + "and 'genafpair'. Default strategy: FFT-NS.", 'maxiterate': 'Specifies how many iterative refinement cycles are ' 'performed after the initial progressive alignment. ' 'By default, no iterative refinement is performed.', From 51057784d0589fdaad73891eeeb079cee3221111 Mon Sep 17 00:00:00 2001 From: Felicia Sandberg Date: Wed, 14 Jan 2026 10:01:49 +0100 Subject: [PATCH 5/6] Limit maxiterate and retree params to range --- q2_alignment/_mafft.py | 8 ++------ q2_alignment/plugin_setup.py | 4 ++-- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/q2_alignment/_mafft.py b/q2_alignment/_mafft.py index 9ddf28a..cec46fd 100644 --- a/q2_alignment/_mafft.py +++ b/q2_alignment/_mafft.py @@ -110,14 +110,10 @@ def _mafft(sequences_fp, alignment_fp, n_threads, parttree, addfragments, if strategy: cmd += [("--" + strategy)] - # --maxiterate is set to 0 by default, so we only pass this argument onto - # MAFFT if it deviates from this value. - if maxiterate not in (None, 0): + if maxiterate is not None: cmd += ['--maxiterate', str(maxiterate)] - # --retree is set to 2 by default, so we only pass this argument onto - # MAFFT if it deviates from this value. - if retree not in (None, 2): + if retree is not None: cmd += ['--retree', str(retree)] if alignment_fp is not None: diff --git a/q2_alignment/plugin_setup.py b/q2_alignment/plugin_setup.py index 59c4c4c..66c7c0a 100644 --- a/q2_alignment/plugin_setup.py +++ b/q2_alignment/plugin_setup.py @@ -19,8 +19,8 @@ "strategy": Str % Choices({ "auto", "nofft", "globalpair", "localpair", "genafpair", }), - "maxiterate": Int, - "retree": Int, + "maxiterate": Int % Range(0, None), + "retree": Int % Range(0, None), } mafft_param_descriptions = { "n_threads": "The number of threads. (Use `auto` to automatically use " From e4180dbb0305c9fe234358c2fac3d1342de66b18 Mon Sep 17 00:00:00 2001 From: Felicia Sandberg Date: Wed, 14 Jan 2026 10:15:12 +0100 Subject: [PATCH 6/6] Update maxiterate and retree defaults; remove affected tests --- q2_alignment/_mafft.py | 8 ++++---- q2_alignment/tests/test_mafft.py | 28 ---------------------------- 2 files changed, 4 insertions(+), 32 deletions(-) diff --git a/q2_alignment/_mafft.py b/q2_alignment/_mafft.py index cec46fd..67356e8 100644 --- a/q2_alignment/_mafft.py +++ b/q2_alignment/_mafft.py @@ -152,8 +152,8 @@ def mafft(sequences: DNAFASTAFormat, parttree: bool = False, large: bool = False, strategy: str | None = None, - maxiterate: int = 0, - retree: int = 2,) -> AlignedDNAFASTAFormat: + maxiterate: int | None = None, + retree: int | None = None,) -> AlignedDNAFASTAFormat: sequences_fp = str(sequences) return _mafft( sequences_fp, None, n_threads, parttree, False, False, large, @@ -169,8 +169,8 @@ def mafft_add(alignment: AlignedDNAFASTAFormat, keeplength: bool = False, large: bool = False, strategy: str | None = None, - maxiterate: int = 0, - retree: int = 2,) -> AlignedDNAFASTAFormat: + maxiterate: int | None = None, + retree: int | None = None) -> AlignedDNAFASTAFormat: alignment_fp = str(alignment) sequences_fp = str(sequences) return _mafft( diff --git a/q2_alignment/tests/test_mafft.py b/q2_alignment/tests/test_mafft.py index 10248c1..bd087bf 100644 --- a/q2_alignment/tests/test_mafft.py +++ b/q2_alignment/tests/test_mafft.py @@ -166,20 +166,6 @@ def test_mafft_maxiterate_flag(self, mock_run_cmd, mock_read): env=None ) - @patch('q2_alignment._mafft.skbio.TabularMSA.read') - @patch('q2_alignment._mafft.run_command') - def test_mafft_maxiterate_not_included_when_default( - self, mock_run_cmd, mock_read): - input_sequences, exp = self._prepare_sequence_data() - mock_read.return_value = exp - - mafft(input_sequences, maxiterate=0) - - call_args, _ = mock_run_cmd.call_args - cmd = call_args[0] - - self.assertNotIn("--maxiterate", cmd) - @patch('q2_alignment._mafft.skbio.TabularMSA.read') @patch('q2_alignment._mafft.run_command') def test_mafft_retree_flag(self, mock_run_cmd, mock_read): @@ -195,20 +181,6 @@ def test_mafft_retree_flag(self, mock_run_cmd, mock_read): env=None ) - @patch('q2_alignment._mafft.skbio.TabularMSA.read') - @patch('q2_alignment._mafft.run_command') - def test_mafft_retree_not_included_when_default( - self, mock_run_cmd, mock_read): - input_sequences, exp = self._prepare_sequence_data() - mock_read.return_value = exp - - mafft(input_sequences, retree=2) - - call_args, _ = mock_run_cmd.call_args - cmd = call_args[0] - - self.assertNotIn("--retree", cmd) - @patch('q2_alignment._mafft.skbio.TabularMSA.read') @patch('q2_alignment._mafft.run_command') def test_mafft_nofft_strategy(self, mock_run_cmd, mock_read):