Iqtree version bump and output fix

[tstoeriko]

PR checklist

Closes #5607

• [x] This comment contains a description of changes (with reason).
• bump version from 2.3.0 to 2.3.3
• include all optional output files (as documented in the IQ-Tree command reference)
• [ ] If you've fixed a bug or added code that should be tested, add tests!
• [x] Follow the naming conventions.
• [x] Follow the input/output options guidelines.
• Ensure that the test works with either Docker / Singularity. Conda CI tests can be quite flaky:
  • For modules:
    • [x] nf-core modules test <MODULE> --profile docker
    • [x] nf-core modules test <MODULE> --profile singularity
    • [x] nf-core modules test <MODULE> --profile conda

[maxulysse]

looking good, but I feel you added some optional files, is there any possibility to test for it too?

[tstoeriko]

looking good, but I feel you added some optional files, is there any possibility to test for it too?

Yes, the module was basically missing all optional output files, likely because there is no centralized documentation of the possible output files. I'll have a look at the tests again and try to include as many of the optional files as possible.

[famosab]

You could also add a stub test while you are doing that :)

[tstoeriko]

You could also add a stub test while you are doing that :)

Will make sure to add stub tests for any additional tests I implement. 👍

[tstoeriko]

This module has now been majorly reworked due to missing inputs and outputs and addition of new tests. I would be grateful for some more feedback.

Major changes:

• Included all optional input and output files. Though there is no centralized documentation of input and output file options, so I cannot guarantee I've caught them all.
• Removed one of the previous inputs (fconst) which is not a file and should be fine to supply via ext.args.
• Made the main alignment input file optional as there are modes which do not require an alignment.
• Wrote tests for nearly all output files.

There are a couple of things I am struggling with:

• The module now has 14 inputs (up from 2), I am a bit worried about usability. Will users just have to adapt?
• I've tried to include relevant file extensions in the meta.yml, but I think IQ-Tree itself doesn't actually check for them. Should I keep the patterns?
• I'd love some feedback on the variable names I've chosen. I think with this huge number of inputs and outputs it is important to make them as intuitive as possible.
• I've spent quite some time on setting up appropriate tests, but I am still missing tests for three specific outputs. The input data currently available do not work for these tests, they are likely not complex enough. I am personally not familiar enough with the corresponding modes to fully troubleshoot the input data requirements. I've tested some data locally, but have not been able to find a suitable small dataset to use. I'm happy to take some time in the future to find a good dataset, but maybe we could get this PR merged for now?

[SPPearce]

You really don't have to test every possible input/output of the tool, so don't worry about missing a few files. Are a lot of the input files mutually exclusive with each other?

[tstoeriko]

You really don't have to test every possible input/output of the tool, so don't worry about missing a few files. Are a lot of the input files mutually exclusive with each other?

I don't think they're mutually exclusive for the most part. Different combinations of input files will result in different modes being run though.

[tstoeriko]

Another thing one could add to the tests is that certain channels should be empty. Otherwise this is great work. Thank you.

Thanks for the suggestion and the thorough review in general, @mahesh-panchal ! I added some tests to confirm that certain optional output channels are empty. If there's nothing else missing, I'll go ahead and get this PR merged.

[mahesh-panchal]

Sounds good. Great work.