12801 bug in gsea module related with multiple gene sets processing by brovolia · Pull Request #12821 · nf-core/modules

brovolia · 2026-08-26T13:56:41Z

I narrowed the removal pattern and used quotation marks, so the white space will not break the processes

pinin4fjords · 2026-08-26T14:06:30Z

    def rpt_label = prefix.replaceAll('\\.$', '') // Remove any trailing dots from prefix when passed as report label, so GSEA doesn't produce double-dotted top-level outputs
    def chip_command = chip ? "-chip $chip -collapse true" : ''
+    // gsea-cli crashes if given more than one -gmx file; callers must invoke this process once per gene set file.
+    def gmx = gene_sets instanceof List ? gene_sets[0] : gene_sets


I don't think silently selecting the first gene set is the right workaround, I think that's worse than erroring when someone supplied the wrong input shape.

We should either explicitly error when a list is supplied, or just remove this and let the fail continue (since any error is at the workflow level)

Agree, I will drop the instanceof List entirely

Actually, I think maybe we just need to make a comma-separated version?

Suggested change

def gmx = gene_sets instanceof List ? gene_sets[0] : gene_sets

def gmx = gene_sets instanceof List ? gene_sets.join(',') : gene_sets

No, in this case, gsea-cli will generate an analysis name by appending the list of collection names. I think it should be fixed at the pipeline-level

pinin4fjords · 2026-08-26T14:07:55Z

    def prefix = task.ext.prefix ?: "${meta.id}"
    def rpt_label = prefix.replaceAll('\\.$', '') // Remove any trailing dots from prefix when passed as report label, so GSEA doesn't produce double-dotted top-level outputs
    def chip_command = chip ? "-chip $chip -collapse true" : ''
+    // gsea-cli crashes if given more than one -gmx file; callers must invoke this process once per gene set file.


We should probably quote the file in the chip_command too

pinin4fjords · 2026-08-26T14:09:06Z

    # Un-timestamp the outputs for path consistency
-    mv ${rpt_label}.Gsea.*/* .
+    # Only rmdir actual directories: the .rpt file's name also matches "*.Gsea.*".
+    mv *.Gsea.*/* .


Is it necessary to have made this glob broader?

yeah.. I will revert it back

pinin4fjords

Ran this for real before going further — pulled the gsea-cli 4.3.2 container and ran the actual script against real GCT/CLS/CHIP data plus two split Hallmark GMT files.

Comma-joined -gmx file1.gmt,file2.gmt: succeeds, combines both files (Got gsets: 50), output dir is plain <rpt_label>.Gsea.<timestamp> — no bracketed name list.
Space-separated (what this module currently produces): also exits 0, but gsea-cli silently keeps only the first file and drops the rest (to parse>hallmark_part1.gmt< got: [hallmark_part1.gmt]). No crash.

So the real bug is silent data loss, not a crash, and join(',') fixes it. Couldn't reproduce the crash or the bracketed-name naming issue — details inline.

pinin4fjords · 2026-08-26T16:00:44Z

    def rpt_label = prefix.replaceAll('\\.$', '') // Remove any trailing dots from prefix when passed as report label, so GSEA doesn't produce double-dotted top-level outputs
    def chip_command = chip ? "-chip $chip -collapse true" : ''
+    // gsea-cli crashes if given more than one -gmx file; callers must invoke this process once per gene set file.
+    def gmx = gene_sets instanceof List ? gene_sets[0] : gene_sets


gsea-cli doesn't crash on multiple files — it silently keeps only the first and drops the rest (confirmed with a real run). It does support multiple files via one comma-joined value (confirmed against GeneSetMatrixMultiChooserParam and a real two-file run combining both, Got gsets: 50).

Suggested change

def gmx = gene_sets instanceof List ? gene_sets[0] : gene_sets

def gmx = gene_sets instanceof List ? gene_sets.join(',') : gene_sets

ok, so here is the question then, do you want one GSEA_GSEA call per contrast with multiple slurm jobs or a multi-collection call per contrast so gsea-cli will handle it? If you want the second option, then I guess conf/modules.config should also be corrected after

I'm not sure there's a good reason to separate them into multiple jobs?

But in any case, it's a slightly orthogonal question. From the module's point of view as a component it should be able to deal with the multiple-input method, since the GSEA CLI can.

pinin4fjords · 2026-08-26T16:00:44Z

    def prefix = task.ext.prefix ?: "${meta.id}"
    def rpt_label = prefix.replaceAll('\\.$', '') // Remove any trailing dots from prefix when passed as report label, so GSEA doesn't produce double-dotted top-level outputs
    def chip_command = chip ? "-chip $chip -collapse true" : ''
+    // gsea-cli crashes if given more than one -gmx file; callers must invoke this process once per gene set file.


This comment's wrong once gmx is comma-joined — worth dropping.

pinin4fjords · 2026-08-26T16:00:44Z

@@ -45,26 +45,32 @@ process GSEA_GSEA {
    def prefix = task.ext.prefix ?: "${meta.id}"
    def rpt_label = prefix.replaceAll('\\.$', '') // Remove any trailing dots from prefix when passed as report label, so GSEA doesn't produce double-dotted top-level outputs
    def chip_command = chip ? "-chip $chip -collapse true" : ''


Since everything else is getting quoted:

Suggested change

def chip_command = chip ? "-chip $chip -collapse true" : ''

def chip_command = chip ? "-chip \"$chip\" -collapse true" : ''

pinin4fjords · 2026-08-26T16:00:44Z

+    # Only rmdir actual directories: the .rpt file's name also matches "*.Gsea.*".
+    mv *.Gsea.*/* .
+    for gsea_dir in *.Gsea.*/; do
+        [ -d "\$gsea_dir" ] && rmdir "\$gsea_dir"
+    done


Tried to reproduce this naming problem — a real comma-joined -gmx across two GMT files produced a plain test_comma3.Gsea.<timestamp> dir, which the original glob already matches. Checked ToolReport.generateReportDir in gsea-desktop v4.3.2 too — built from --rpt_label + tool name + timestamp only. Suggest reverting once gmx is comma-joined:

Suggested change

# Only rmdir actual directories: the .rpt file's name also matches "*.Gsea.*".

mv *.Gsea.*/* .

for gsea_dir in *.Gsea.*/; do

[ -d "\$gsea_dir" ] && rmdir "\$gsea_dir"

done

mv ${rpt_label}.Gsea.*/* .

brovolia added 2 commits August 26, 2026 15:42

using quoting for file names if possible

38e4e31

dropping comments

8c1d769

brovolia linked an issue Aug 26, 2026 that may be closed by this pull request

Bug in gsea module related with multiple gene sets processing #12801

Open

2 tasks

github-actions Bot added the size/s label Aug 26, 2026

pinin4fjords reviewed Aug 26, 2026

View reviewed changes

	def gmx = gene_sets instanceof List ? gene_sets[0] : gene_sets
	def gmx = gene_sets instanceof List ? gene_sets.join(',') : gene_sets

	def chip_command = chip ? "-chip $chip -collapse true" : ''
	def chip_command = chip ? "-chip \"$chip\" -collapse true" : ''

Uh oh!

Conversation

brovolia commented Aug 26, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pinin4fjords left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants