Skip to content

Conversation

@cmatKhan
Copy link

@cmatKhan cmatKhan commented May 14, 2025

this differs from #17 by attempting to split the non-interproscan steps contained in the unifire docker image into components of a subworkflow.

there is an in-process bioconda recipe for unifier at:

bioconda/bioconda-recipes#56175

@nf-core-bot
Copy link
Member

Warning

Newer version of the nf-core template is available.

Your pipeline is using an old version of the nf-core template: 3.2.0.
Please update your pipeline to the latest version.

For more documentation on how to update your pipeline, please see the nf-core documentation and Synchronisation documentation.

@heuermh
Copy link
Contributor

heuermh commented May 14, 2025

Your pipeline is using an old version of the nf-core template: 3.2.0.

I haven't been following development closely this week, but shouldn't this pull request already have the 3.2.1 template changes in? They were merged in #26

@cmatKhan cmatKhan marked this pull request as ready for review May 19, 2025 00:53
@cmatKhan
Copy link
Author

This is, if you are interested in running it now, ready to go. The tests won't pass, except for the stubs -- the input data (the Unirule URML files, etc) are large. They will need to be reduced to minimal test size before they can be put up on the nf-core test repo.

However, I can provide (or tell you how to get -- easy) the test data to run the tests locally if you're interested. It is also already documented in the meta.yml in each of the modules.

The pirsr cmd produces an error, with the input data provided by Uniprot, in the Uniprot official container. And, it produces the same error in the bioconda container, too. The subworkflow requires that a parameter be provided, unifire_skip, so for the time being be sure to set that to unifire_skip = "pirsr". I have an issue report out to Uniprot about this.

I'm waiting to hear back from Uniprot before getting the Unifire bioconda-recipe merged. But, it is ready to go and you can get the docker image and conda env here. Hence, the container in each of the unifire modules is set to my personal dockerhub.

The meta data files are all filled out. The only thing that will need to be done before pushing the modules and subworkflow to nf-core will be to pare down the URML, etc rules files so that they are small enough to be pushed up to the nf-core test repo.

@cmatKhan cmatKhan force-pushed the unifire_subworkflow branch from 916c574 to adfd55c Compare May 19, 2025 01:07
@olgabot
Copy link
Collaborator

olgabot commented May 19, 2025

I haven't been following development closely this week, but shouldn't this pull request already have the 3.2.1 template changes in? They were merged in #26

Re the nf-core template, addressing that here: #49 (now merged!)

EDIT: Update quote, state the version PR is now merged

…sion

Update erroneously downgraded nf-core version
@olgabot
Copy link
Collaborator

olgabot commented May 19, 2025

@cmatKhan From talking to @maxulysse at the hackathon, some tests are better than no tests! Let's do stub tests for now. Before we cut a release, we can do full tests with the CI: .github/workflows/awsfulltest.yml. Please go ahead and add the full data to the conf/test_full.config file so we can have a complete end-to-end test. Thank you so much for your help!

@maxulysse
Copy link
Member

some tests are better than no tests

I stand by this.
We unfortunately don't live in a perfect world.
We do what we can, but we don't forget what needs to be done.

@cmatKhan
Copy link
Author

cmatKhan commented May 20, 2025

Sounds good -- I am going to wait on hearing back from Uniprot re: pirsr.

My plan right now is the following (open to feedback and revision of course):

edit -- I didn't see the comment about the aws test, so:

  1. Add the input data paths to the aws test config

  2. Since this currently works, I'll leave it open here in case anyone wants to use it, eg if the interproscan step gets added, it would be easy to rebase this PR and then add the unifire subworkflow into the main workflow in a branch.

  3. Once the solution for pirsr comes back from uniprot, I'll open PRs in nf-core/module and /subworkflows with the data tests commented out and just the stubs. I'll make an issue here to create minimal input files and that will be a longer term (maybe another hackathon?) goal.

  4. Once the nf-core modules and subworkflow are merged, I'll open another PR here that adds the schema docs and config. We'll just keep it open until interproscan gets added and then I'll put the finishing touches on integrating unifire in the main.

@cmatKhan
Copy link
Author

Requests/comments from unifire/uniprot:

  1. keep update lineage in the unifire bioconda recipe (the way it is currently)

  2. add an optional interproscan step to the subworkflow. If a fasta is passed, then run interproscan. If a interproscan xml is passed, skip it. This is the way the unifire workflow currently is. I am torn -- it woudl require that interproscan always come along with the unifire subworkflow. After agreeing to this, I'm re-thinking it and leaning towards no

@cmatKhan
Copy link
Author

cmatKhan commented May 28, 2025

The unirule annotation step, at least, has a random order to its output. Eg

Where among other differences, this line is in this order in one output

...
UR001995901	A0A009EAR0	xref.GO	GO:0005737	
UR000084156	A0A009EAR0	xref.GO	GO:0005737		
...

and in this order in the other

...
UR000084156	A0A009EAR0	xref.GO	GO:0005737		
UR001995901	A0A009EAR0	xref.GO	GO:0005737	
...

can't test with md5sum as a result. Testing by number of lines and searching for some matching patterns instead

@olgabot
Copy link
Collaborator

olgabot commented Jun 2, 2025

Check out this webinar talk about UniProt's automated annotation pipeline: https://www.ebi.ac.uk/training/events/automated-annotation-uniprot/

@olgabot olgabot mentioned this pull request Jun 24, 2025
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

No open projects
Status: No status

Development

Successfully merging this pull request may close these issues.

5 participants