fix bugs in simpleleaf_index.nf and config files

[haibol2016]

PR checklist

• This comment contains a description of changes (with reason).
• If you've fixed a bug or added code that should be tested, add tests!
• If you've added a new tool - have you followed the pipeline conventions in the contribution docs
• If necessary, also make a PR on the nf-core/scrnaseq branch on the nf-core/test-datasets repository.
• Make sure your code lints (nf-core pipelines lint).
• Ensure the test suite passes (nextflow run . -profile test,docker --outdir <OUTDIR>).
• Check for unexpected warnings in debug mode (nextflow run . -profile debug,test,docker --outdir <OUTDIR>).
• Usage Documentation in docs/usage.md is updated.
• Output Documentation in docs/output.md is updated.
• CHANGELOG.md is updated.
• README.md is updated (including new tool citations and authors/contributors).

[github-actions]

Warning

Newer version of the nf-core template is available.

Your pipeline is using an old version of the nf-core template: 3.0.2.
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.

[grst]

Could you please describe in more detail what you are trying to accomplish here? Please be aware that simpleaf has been refactored and updated in #424, so maybe some of your changes might not be necessary anymore.

[nictru]

I think @haibol2016 did a lot of different things here, some of which are good, some are bad and some are not necessary.

Some remarks

modules.config

Concerning the modules.config update: I agree that the if statements should be removed; the warnings that arise if a withName selector does not match any process have been disabled by default for some time, afaik. Also, I encountered some inconsistent behaviors with these if statements. That's why I started this thread in the maintainer's team.

Updating files in nf-core/

These are shared nf-core modules and cannot be updated within the pipeline repo. If you want to perform changes, you need to do this using the modules repo. Then, we can add it to the pipeline using the nf-core CLI toolkit.

Adding def prefixes to definitions in script blocks

This is something weird about nextflow, but we must comply with it: Adding the def prefix makes variables local, so they cannot be used within templates. Without the def prefix, everything works fine. So please just leave it as it is.

Editing base.config and certain parts of nextflow.config

These sections are defined by the nf-core pipeline template and should not be modified if there are no good reasons. I don't see such reasons here. If you have some, let's discuss.

Conclusion

While there are some good parts about PR, most of it breaks things rather than makes them better. Generally, it is a good idea to discuss planned changes first in an issue or via Slack so that misunderstandings can be discussed before making an effort to do the implementations.

For now, I suggest reducing this PR to updating modules.config and then discussing the additional change proposals you have in issues or via Slack. Afterward, you can create more targeted PRs so that maintainers don't get overwhelmed by the diversity of changes.

[haibol2016]

@grst

I may have made some mistakes in my branch. I don't know that a variable with "def" is not accessible in a "template file".
However, there is definitely a bug here: https://github.com/nf-core/scrnaseq/blob/dev/modules/local/simpleaf_index.nf#L31-L48. You can ignor my other changes.

I tried to build a index to use "salmon" as the aligner. If a user provides "params.genome_fasta", "params.transcript_fasta", and "params.gtf", the module will fail, because genome_fasta (should be specified together with gtf) is not compatible wiht transcript_fasta.

Instead the code should be as follows.

`def seq_inputs = (params.transcript_fasta) ? "--refseq $transcript_fasta" : "--fasta $genome_fasta --gtf $transcript_gtf"
"""
# export required var
export ALEVIN_FRY_HOME=.
export NUMBA_CACHE_DIR=.

# prep simpleaf
simpleaf set-paths

# run simpleaf index
simpleaf \\
    index \\
    --threads $task.cpus \\
    $seq_inputs \\
    $args \\
    -o salmon`

Haibo

[nictru]

As I now had a closer look at #424, I can say that it tackles the transcript_fasta/genome_fasta problem. As #424 is a more sustainable fix (also moves some modules to nf-core/modules and contains a complete rewrite of the alevin subworkflow), I will close this PR.

Still, @haibol2016 thanks for the contribution! We're always happy to see new contributors, but maybe next time open an issue or ask on slack first :)

[haibol2016]

Sure. Thanks for fix the issues. Best, Haibo

…

On Tue, Feb 11, 2025 at 3:27 AM Nico Trummer ***@***.***> wrote: As I now had a closer look at #424 <#424>, I can say that it tackles the transcript_fasta/genome_fasta problem. As #424 <#424> is a more sustainable fix (also moves some modules to nf-core/modules and contains a complete rewrite of the alevin subworkflow), I will close this PR. Still, @haibol2016 <https://github.com/haibol2016> thanks for the contribution! We're always happy to see new contributors, but maybe next time open an issue or ask on slack first :) — Reply to this email directly, view it on GitHub <#429 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ALR5WD5GCZDRP5JLKEW73ED2PGYALAVCNFSM6AAAAABWMQOGSKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMNJQGEYDSMBQGI> . You are receiving this because you were mentioned.Message ID: ***@***.***>