bgruening / galaxytools

:microscope::books: Galaxy Tool wrappers
MIT License
115 stars 222 forks source link

Mafft Changes #1337

Closed elischberg closed 4 months ago

elischberg commented 9 months ago

Fixes:

Fixes need to be done:

bgruening commented 8 months ago

You need to rebase this PR, please.

elischberg commented 8 months ago

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 commented 8 months ago

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

elischberg commented 8 months ago

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 commented 8 months ago

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?

bgruening commented 7 months ago

Do you test this locally with planemo test --biocontainers ...?

elischberg commented 7 months ago

I tested it with planemo test but not with the --biocontainer argument. What does change by adding it?

bgruening commented 7 months ago

--biocontainers is what is running on CI. So you should get the same errors locally what you get here. Containers are just more isolated and produce more similar results.

elischberg commented 7 months ago

Ok, thank you for the explanation. I tried it with --biocontainers locally and everything was fine. Here it says that Test 4 is a failure. How can this be?

bgruening commented 7 months ago

Unlikely. Have you commited everything?

What does git status . and git diff . show in your maff directory?

elischberg commented 7 months ago

I commited and pushed everything and all tests passed locally.

elischberg commented 7 months ago

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 commented 7 months ago

grafik

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 commented 7 months ago

GreeeennnnnnnnnnnN!

elischberg commented 7 months ago

Very good. (:

bgruening commented 7 months ago

ping @wm75

wm75 commented 4 months ago

@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 commented 4 months ago

@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 commented 4 months ago

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

bgruening commented 4 months ago

failing ...