Mafft Changes

[elischberg]

Fixes:

• Fix datatype selection matrix choice
• Autodetection matrix definition
• Update version number

Fixes need to be done:

• according alignment strategies with help section
• insert arguments
• tests correction

[bgruening]

Oh wow, is 7.52 really bigger/newer than 7.508?

[wm75]

The latest version is actually 7.520. @elischberg I don't think you can just drop that trailing zero.

[elischberg]

Oh my bad. I thought, the dot is a comma.

[bgruening]

You need to rebase this PR, please.

[elischberg]

Tried for the groupsize param to get a max limit through a validator with the sequence metadata, but I think it then ignored every inserted integer of the groupsize param. Solved it right now with a condition in the command part of mafft.xml.

[bgruening]

This should work https://github.com/galaxyproject/tools-iuc/blob/ef564483fa8dc267c599af8745aab4e03b0c57e0/tools/ampvis2/venn.xml#L32

[elischberg]

In venn.xml it is a fix maximum. Unfortunately not appliable in the groupsize case with a variable maximum. We solved it with a warning for users, when they use a too large number.
But thanks!

[elischberg]

There are difficulties with the groupsize parameter. I don't know, how to change the empty value of $cond_flavour.guidetree.partetree_selection.groupsize into the calculated $sequence_count. Do you have an idea?

[wm75]

Although the error:

'>' not supported between instances of 'NoneType' and 'int'

looks like it's coming from here, you can completely remove the whole section and when you don't enter anything for the optional integer param still get it in exactly the same form.

I think this comes from an internal attempt to compare the None to the "min" and "max" values of the param =>
There's just no way to combine min, max and optional="true" for an int param.

As discussed try to use -1 instead and explain it in the param help.

[elischberg]

-1 isn't working. Then MAFFT gives an error like: Specify groupsize!
I think it is an internal function of MAFFT, if one adding a larger number than input sequences, then it shows -1 but it takes one more than the number of input sequences.

I'm just wondering, because there was a moment, when it worked with the comparison.

[elischberg]

I can remove the section but then we dont have a warning in die stoud. I can simply mention it somewhere else?

[elischberg]

git status: clean worktree and git diff:nothing.
Its only Test 4 which is online a failure with the error of a different output.

[bgruening]

So the results for test 4 are still different.
If you run this already in Containers, then the only explanation I have is that this test is not deterministic and generates a different output based on some other wired stuff.

But the changes look vastly different :(

[bgruening]

GreeeennnnnnnnnnnN!

[elischberg]

Very good. (:

[bgruening]

ping @wm75

[wm75]

Exposing these settings unconditionally makes no sense since they are part of the "flavour" definitions.
If a user selects a pre-defined flavour above and you're overwriting some of that flavour's settings here, it's not that flavour anymore.

@elischberg I've opened another PR against your branch that fixes this and a few other things.
Once that's merged you will have to regenerate all test data except for the first test (for which the settings here happen to be that of the flavour).
I guess it would be a good idea to generate all test data with mafft flavour commands on the command line to make sure the wrapper produces matching outputs.

[wm75]

@elischberg cool!
What about test 4, which you previously changed to check only for a few lines of output.
Is it by chance possible to make this one stricter again now that --threadit is set to 0?

[elischberg]

@elischberg cool! What about test 4, which you previously changed to check only for a few lines of output. Is it by chance possible to make this one stricter again now that --threadit is set to 0?

Going to try (:

[wm75]

The mafft_auto_result.aln test file was also used by the mafft_add test, which is now failing.

[bgruening]

failing ...

[wm75]

Thanks for the hard work on this complex wrapper @elischberg !