Skip to content

Fix Comet version reporting in pmultiqc software table#699

Open
Copilot wants to merge 4 commits intodevfrom
copilot/fix-comet-version-null-in-pmultiqc
Open

Fix Comet version reporting in pmultiqc software table#699
Copilot wants to merge 4 commits intodevfrom
copilot/fix-comet-version-null-in-pmultiqc

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 13, 2026 •
edited
Loading

In dev, pmultiqc shows Comet version as null because the version command in the COMET module targets a binary name that is not available in the current OpenMS container. This updates version extraction to use the bundled Comet executable and parse its output reliably.

  • Root cause

    • modules/local/openms/comet/main.nf used comet for version collection, but the container exposes Comet at /opt/OpenMS/thirdparty/Comet/comet.exe.

  • Change

    • Replaced the Comet versions.yml command to call the bundled executable directly.
    • Hardened parsing to handle leading whitespace and capture only the first matching version line.

  • Impact

    • nf_core_quantms_software_mqc_versions.yml now carries a concrete Comet version value, so pmultiqc no longer renders null for Comet.
Comet: $(/opt/OpenMS/thirdparty/Comet/comet.exe 2>&1 \
  | grep -m1 -E "^[[:space:]]*Comet version.*" \
  | sed 's/^[[:space:]]*Comet version //g' \
  | sed 's/"//g')

Copilot AI linked an issue Apr 13, 2026 that may be closed by this pull request
quantms comet version is null in pmultiqc #698
Open
@codacy-production
Copy link
Copy Markdown

codacy-production bot commented Apr 13, 2026 •
edited
Loading

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

TIP This summary will be updated as you push new changes. Give us feedback

Copilot AI changed the title [WIP] Fix comet version being null in pmultiqc Fix Comet version reporting in pmultiqc software table Apr 13, 2026
Copilot AI requested a review from ypriverol April 13, 2026 15:43
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 14, 2026 •
edited
Loading

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

Posted for pipeline commit 7386ee4

+| ✅ 109 tests passed       |+
#| ❔  19 tests were ignored |#
#| ❔   1 tests had warnings |#
!| ❗   3 tests had warnings |!
Details

❗ Test warnings:

  • pipeline_todos - TODO string in nextflow.config: Specify any additional parameters here
  • pipeline_if_empty_null - ifEmpty(null) found in /home/runner/work/quantms/quantms/subworkflows/local/dda_id/main.nf: _ ch_software_versions = ch_software_versions.mix(PHOSPHO_SCORING.out.versions.ifEmpty(null))
    _
  • pipeline_if_empty_null - ifEmpty(null) found in /home/runner/work/quantms/quantms/subworkflows/local/id/main.nf: _ ch_software_versions = ch_software_versions.mix(PHOSPHO_SCORING.out.versions.ifEmpty(null))
    _

❔ Tests ignored:

❔ Tests fixed:

✅ Tests passed:

Run details

  • nf-core/tools version 3.5.2
  • Run at 2026-04-14 18:56:49

@ypriverol ypriverol marked this pull request as ready for review April 14, 2026 10:55
ypriverol and others added 2 commits April 14, 2026 11:56
@ypriverol @claude
fix: add fallback for Comet version extraction
ebc09d1 
Add `|| echo "unknown"` so versions.yml gets a safe value if the
bundled executable path ever changes between container rebuilds,
rather than an empty/broken entry.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@ypriverol @claude
fix: remove || echo "unknown" fallback from Comet version command
7386ee4 
With Nextflow's set -euo pipefail, the || operator inside $() does not
behave reliably when a pipeline fails: the subshell exits early (set -e),
leaving $() empty, and echo "unknown" leaks into the heredoc as a bare
word, producing invalid YAML ("unknown" without a colon on its own line).

Dropping the fallback is safe because:
- The correct binary path is already the real fix (comet was not on PATH)
- /opt/OpenMS/thirdparty/Comet/comet.exe exists and produces the version
- An empty YAML value is valid if the command ever fails

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ypriverol ypriverol Awaiting requested review from ypriverol

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

Assignees

@ypriverol ypriverol

Copilot code review Copilot

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

quantms comet version is null in pmultiqc

2 participants

@ypriverol