Skip to content

added cmscan process used in rfam pipeline - #43

Open
Dishalodha wants to merge 3 commits into
mainfrom
disha/rfam_modules
Open

Dishalodha wants to merge 3 commits into
mainfrom
disha/rfam_modules

Conversation

@Dishalodha

Copy link
Copy Markdown
Collaborator

Adding the cmscan module used in rfam

@markquintontulloch markquintontulloch left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just a slight query about placement of the version reporting logic

@Dishalodha

Copy link
Copy Markdown
Collaborator Author

thanks @markquintontulloch, I have seen topic being used in nf-core modules now. I think it is to call the version information from modules and report it in one channel. You can now call the versions with channel.topic('versions') or version_files = channel.topic('versions').collect()
doc: https://docs.seqera.io/nextflow/tutorials/topic-channels

@markquintontulloch

Copy link
Copy Markdown
Contributor

@Dishalodha sorry, I don't think I was clear. I wasn't questioning the use of a versions topic. I was suggesting that we could do away with the creation of the versions.yml file as an intermediate that's read by the topic definition, and use an eval directly in the definition. That appears to be the approach suggested in the guide you linked to above (in the 'Using eval outputs' section). However, whereas the example in that guide discards task.process as the first entry in the topic tuple, I think we should keep that in order to maintain the same structure as the modules from nf-core.

So, in the case of this module, I think the versions topic should be defined as:
topic: tuple("${task.process}", 'infernal', eval("cmscan -h 2>&1 | sed -n 's/.*# INFERNAL \\([^ ]*\\).*/\\1/p'")) >> 'versions'

and the END_VERSIONS block should be dropped from the script section.

@markquintontulloch markquintontulloch left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants