I opened a PR against modules for RiboCode but I am quite rusty in terms of the latest nf-core guidelines so may need some help getting it in shape. https://github.com/nf-core/modules/pull/9485

Thanks! From what I can see, the test failures are snapshot-related due to PDF outputs. Since PDFs generate variable md5sums between runs (even with identical content), replace md5-based assertions with alternative checks for those types of files in your tests. That should sort it out!

Thanks Ira! I’ll find an example of a module that outputs a pdf and copy that tomorrow

Here's a quick one with PDFs for reference, not sure if this one passes current testing guidelines https://github.com/nf-core/hlatyping/blob/master/modules/nf-core/optitype/tests/main.nf.test.snap

Thats a great help. Thank you Isaac

Reviewing now

I was just trying to look into it in more detail before asking for your help. I am stumped on on why some conda are failing and there is another with a subworkflow test failing that, to the best of my knowledge, I didnt touch. I have however noticed that I haven't addressed all of your requested changes yet. I will knock them out this morning and get back to you for help with the others

OK, so the issue is that conda environments aren't static. The transitive dependencies (dependencies of dependencies) drift. That means that over time the snapshots between Docker and Conda no longer match, which can cause variation in results. The community is going to address this soon via 'Conda lock files' (which Wave and Seqera containers can make for us). We should fix this by pinning Python in the Biconda recipe I think. Here's my little buddy's summary:

⏺ This is a Python 3.10+ compatibility issue with the pyfasta dependency.

  The key error is:
  ImportError: cannot import name 'Mapping' from 'collections'

  What's happening:

  In Python 3.10, collections.Mapping was removed (it had been deprecated since Python 3.3). The correct import is now collections.abc.Mapping.

  The dependency chain is:
  1. RiboCode → pyfasta → tries from collections import Mapping → fails

  The root cause:

  The pyfasta package is outdated and hasn't been updated for Python 3.10+ compatibility. The conda environment is using Python 3.10, which breaks this old import pattern.

  Potential fixes:

  1. Pin Python < 3.10 in the environment.yml:
  dependencies:
    - python=3.9
    - ribocode
  2. Use a patched pyfasta - there may be a fork that fixes this, or the team could pin a specific version that works
  3. Check if RiboCode has a newer version that uses an updated pyfasta or alternative dependency

  This is a common problem with older bioinformatics tools that depend on unmaintained Python packages. Pinning to Python 3.9 is usually the quickest workaround.

That makes sense. Thank you. I am going through your comments again right now

This is just the spectre of unmaintained bioinformatics software I'm afraid. I see ribocode itself hasn't had a release since 2021

The only think I have left to resolve is the topics instead of

versions.yml

Were you happy with how I defined the versions? example there is no

RiboCode --version

Think you're wrong there:

❯ docker run --rm <http://quay.io/biocontainers/ribocode:1.2.15--pyhfa5458b_0|quay.io/biocontainers/ribocode:1.2.15--pyhfa5458b_0> RiboCode --version
1.2.14

I think I havent woken up properly.. I only have the echo in stubs

(though the reported version is wrong I see....)

Thank you. That is good to know

I think someone messed up, both versions are definitely in PyPi. Probably someone forgot to update the version in the released code

Set up Bioconda channels:

conda config --add channels bioconda
conda config --add channels conda-forge
conda config --set channel_priority strict

See https://bioconda.github.io/#usage

@Jonathan Manning I think I finally have it in ok shape. Just need the bioconda fix, right? https://github.com/nf-core/modules/pull/9485

Yep, I'll push some changes for that now if that's OK

Thank you very much. I needed the hand holding on this one

Bioconda stuff can be a pain

Having a look now!

OK, you're good to go too- tests should pass in a few mins ✅

Merged! Thanks again Jonathan.

Are you planning to work on the pipeline-level integration?

Yup I was just about to ask you about that. I have started including it the same way we have Ribotricer and RiboTISH but we may want to consider a subworkflow instead? Especially as I plan to add at least 3 more tools

Yep, subworkflow might be good. Make it a 'local' one while you work it up, get tests running etc, and then migrate to nf-core/modules last thing. Do prompt me for assistance when needed, I know this is a priority for @Felix Krueger

Perhaps we get it merged copying the current formatting for now then and move to subworkflow when the other modules are ready. They aren't even in bioconda for the most part so there is so social engineering to be done to get the tool devs to package their tools better

Sure, we can mop up later

I think the bones of the integration is done. I am just running the full test before making a PR. I suspect I will run into issues if the test samples aren't periodic enough for RiboCodes liking. https://github.com/nf-core/riboseq/tree/feat/add-ribocode

Can you make a draft PR so we can see the changes?

I thought that might kick off the tests so I was holding off: https://github.com/nf-core/riboseq/pull/136

FYI if you put

[skip ci]

in your commit messages it won't trigger CI. It's something I don't do enough, but it's handy for polar bear saving until you're done mucking about

Ah. Another good tip. Thank you

Hey! I’m not super familiar with how ribocode works, but I wonder - would the results be reliable if the periodicity requirement it was designed with in mind is not satisfied? And if so, would it be wiser to output an informative error/log instead and not run the tool? Sorry if my q is naive

No you're spot on. It'd weaken the confidence in the calls. I do think their standards are very high though. Will we need to change the test data for full test then?

So, the basic (non full size) tests are really only there to make the tools run in some way, the output does not need to be at all meaningful in a biological sense. I think I reduced the depth of the reads as much as I could get away with at the time (though they're still not tiny). The hack is perfectly normal in that sense. I'd be more sad if you had to do it to make the full size tests run.

Unfortunately this is with

-profile test_full

. I copied the Ribotricer implementation so RiboCode isnt run in the basic tests

Ahh, we shouldn't necessarily stop ribocode from running just because we stopped ribotricer. That tool just didn't run at all with the reduced depth sequencing. If Ribocode runs (even if the output is nonsense) we should include it because it helps us monitor differences in behaviour between PRs. As for

test_full

, if one of you knows a nice clean public dataset we could use instead I don't see a problem with switching.

Perhaps something of note is that RiboCode doesnt actually error

Command output:
  Error, can not determine the P-site locations.Please check your input parameters and try again.

And the process fails because the outputs are missing. We could make the outputs optional?

Well, that's just crappy software design and it should error

Yes that sounds good. That will let us give an informative error message too

Ha, yep, same deal. Bioinformaticians are my people, but I do wonder about us as a species sometimes 😄

Okay hope to have a PR for you this morning

Great, thanks. If there's anything you haven't got time for I can have Claudio take it the last mile.

https://github.com/nf-core/modules/pull/9581 This is all I managed. Could Claudio have a look?

OK @Jack Tierney, we fixed it up. One of the modules wasn't actually failing when it should, so that's fixed and I've also added tests to make sure the failure modes keep working. I've approved, assuming the tests pass in CI (if they don't I'll fix them) feel fee to merge