Skip to content

Harmonize the feature name columns params#646

Open
delfiterradas wants to merge 1 commit intonf-core:devfrom
delfiterradas:diff_column
Open

Harmonize the feature name columns params#646
delfiterradas wants to merge 1 commit intonf-core:devfrom
delfiterradas:diff_column

Conversation

@delfiterradas
Copy link

@delfiterradas delfiterradas commented Feb 18, 2026 •
edited
Loading

Closes #618

  • Remove the --differential_feature_name_column parameter and consolidate its functionality into --features_name_col.

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/differentialabundance branch on the nf-core/test-datasets repository.
  • Make sure your code lints (nf-core lint).
  • Ensure the test suite passes (nf-test test main.nf.test -profile test,docker).
  • 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).

@delfiterradas delfiterradas self-assigned this Feb 18, 2026
@github-actions
Copy link

github-actions bot commented Feb 18, 2026 •
edited
Loading

nf-core pipelines lint overall result: Passed ✅ ⚠️

Posted for pipeline commit 1416379

+| ✅ 381 tests passed       |+
#| ❔  10 tests were ignored |#
#| ❔   1 tests had warnings |#
!| ❗  20 tests had warnings |!
Details

❗ Test warnings:

  • readme - README contains the placeholder zenodo.XXXXXXX. This should be replaced with the zenodo doi (after the first release).
  • pipeline_todos - TODO string in nextflow.config: Update the field with the details of the contributors to your pipeline. New with Nextflow version 24.10.0
  • pipeline_todos - TODO string in base.config: Check the defaults for all processes
  • pipeline_todos - TODO string in main.nf: Optionally add in-text citation tools to this list.
  • pipeline_todos - TODO string in main.nf: Optionally add bibliographic entries to this list.
  • pipeline_todos - TODO string in main.nf: Only uncomment below if logic in toolCitationText/toolBibliographyText has been filled!
  • schema_lint - Parameter input is not defined in the correct subschema (input_output_options)
  • schema_description - No description provided in schema for parameter: deseq2_seed
  • schema_description - No description provided in schema for parameter: dream_p_value
  • schema_description - No description provided in schema for parameter: dream_lfc
  • schema_description - No description provided in schema for parameter: dream_confint
  • schema_description - No description provided in schema for parameter: dream_proportion
  • schema_description - No description provided in schema for parameter: dream_stdev_coef_lim
  • schema_description - No description provided in schema for parameter: dream_trend
  • schema_description - No description provided in schema for parameter: dream_robust
  • schema_description - No description provided in schema for parameter: dream_winsor_tail_p
  • schema_description - No description provided in schema for parameter: dream_ddf
  • schema_description - No description provided in schema for parameter: dream_reml
  • schema_description - No description provided in schema for parameter: dream_apply_voom
  • schema_description - No description provided in schema for parameter: dream_adjust_method

❔ Tests ignored:

  • files_exist - File is ignored: assets/multiqc_config.yml
  • nextflow_config - Config default ignored: params.report_file
  • nextflow_config - Config default ignored: params.logo_file
  • nextflow_config - Config default ignored: params.css_file
  • nextflow_config - Config default ignored: params.citations_file
  • files_unchanged - File ignored due to lint config: .github/CONTRIBUTING.md
  • files_unchanged - File ignored due to lint config: assets/nf-core-differentialabundance_logo_light.png
  • files_unchanged - File ignored due to lint config: docs/images/nf-core-differentialabundance_logo_light.png
  • files_unchanged - File ignored due to lint config: docs/images/nf-core-differentialabundance_logo_dark.png
  • multiqc_config - multiqc_config

❔ Tests fixed:

✅ Tests passed:

Run details

  • nf-core/tools version 3.5.1
  • Run at 2026-02-18 18:10:10

@delfiterradas delfiterradas marked this pull request as ready for review February 18, 2026 18:50
@pinin4fjords
Copy link
Member

pinin4fjords commented Feb 19, 2026

Could you flesh out the description here (and on #618)? It's hard to evaluate without more context on the reasoning.

I think there's an issue with the limma_soft profile. features_name_col and differential_feature_name_column aren't always interchangeable - one refers to the column in the features/annotation table, the other to the column as it appears in the differential results output. For limma_soft these were intentionally different:

  • features_name_col: Gene Symbol (column in the SOFT features table)
  • differential_feature_name_column: Symbol (column in limma's topTable output)

After this PR, the report looks for Gene Symbol in the DE results, won't find it (limma outputs Symbol), and falls back to showing probe IDs on volcano/upset plots instead of gene symbols.

The other profiles happen to have matching values so they're fine, but this case shows the two parameters existed for a reason. Could you either restore the SOFT handling, or ensure the column name gets harmonized earlier in the pipeline so they always match?

Also, please add a CHANGELOG entry for the parameter removal.

@grst
Copy link
Member

grst commented Feb 20, 2026

Ok, thanks for providing that additional context. If it's not too complicated I think we should address this and make sure that the column names from SOFT files get carried over correctly and automatically.

@delfiterradas
Copy link
Author

delfiterradas commented Feb 20, 2026

Thanks, I will look into this and make the changes.

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

Reviewers

@pinin4fjords pinin4fjords Awaiting requested review from pinin4fjords

@grst grst Awaiting requested review from grst

At least 1 approving review is required to merge this pull request.

Assignees

@delfiterradas delfiterradas

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@delfiterradas @pinin4fjords @grst

Comments