Skip to content

update onsite#40

Merged
daichengxin merged 9 commits into
bigbio:mainfrom
daichengxin:main
Jun 13, 2026
Merged

update onsite#40
daichengxin merged 9 commits into
bigbio:mainfrom
daichengxin:main

Conversation

@daichengxin

@daichengxin daichengxin commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

Pull Request

Description

Checklist

  • Module follows nf-core standards
  • main.nf includes process definition
  • meta.yml includes complete documentation
  • environment.yml specifies dependencies
  • Tests are included
  • Code is formatted (prettier)
  • CI checks pass

Module Type

  • New module
  • Module update
  • Bug fix
  • Documentation

Related Issues

Closes #

Summary by CodeRabbit

  • Chores

    • Upgraded ONSITE dependency and container images to v0.0.3.
    • CI now fetches onsite test parquet data during module tests.
  • Changes

    • ONSITE now produces idparquet files instead of idXML for all algorithms (AScore, PhosphoRS, LucXor).
    • Tests and test data updated to use idparquet inputs and validate idparquet outputs.

@qodo-code-review

Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@coderabbitai

coderabbitai Bot commented Jun 13, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@daichengxin, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 51 minutes and 50 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 95364f89-df5c-424a-a05f-bfdc7b7355d9

📥 Commits

Reviewing files that changed from the base of the PR and between f44bde1 and 5e0edb0.

📒 Files selected for processing (3)
  • .github/workflows/ci.yml
  • .gitignore
  • tests/config/test_data.config
📝 Walkthrough

Walkthrough

Update ONSITE to pyonsite 0.0.3 and migrate module IO from idXML to idparquet: environment and container refs updated, Nextflow process outputs and per-algorithm -out targets changed, meta.yml I/O patterns adjusted, tests/test-data and CI download step switched to idparquet.

Changes

ONSITE Module v0.0.3 Update

Layer / File(s) Summary
Dependency and container setup
modules/bigbio/onsite/environment.yml, modules/bigbio/onsite/main.nf
Conda environment bumped to pyonsite=0.0.3; Nextflow process container references updated (Singularity fallback and Docker image) and output: updated to emit ${prefix}_*.idparquet instead of ${prefix}_*.idXML.
ONSITE algorithm implementations
modules/bigbio/onsite/main.nf
Algorithm branches ascore, phosphors, and lucxor now write -out targets to ${prefix}_*.idparquet instead of .idXML.
Module specification
modules/bigbio/onsite/meta.yml
Input id_file pattern and output ptm_in_id_onsite pattern changed from *.idXML to *.idparquet.
Test implementation and data
modules/bigbio/onsite/tests/main.nf.test, tests/config/test_data.config
Tests updated to source params.test_data['proteomics']['onsite']['idparquet'] and assert _*.idparquet outputs; test_data config mzML and idparquet resource URLs updated.
CI workflow
.github/workflows/ci.yml
Add conditional CI step to download onsite .parquet test files when testing bigbio/onsite.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • bigbio/nf-modules#17: Both PRs touch the bigbio/onsite module’s Conda/container setup by bumping the pyonsite dependency/image versions.
  • bigbio/nf-modules#25: Modifies ONSITE output filename/prefix handling similar to this PR.
  • bigbio/nf-modules#12: Introduced original ONSITE module that emitted idXML outputs prior to this idparquet migration.

Suggested labels

Review effort 2/5

Suggested reviewers

  • ypriverol
  • jpfeuffer

Poem

🐰 I hopped from XML to parquet light,
Bumped a version, set outputs right.
Tests and CI fetched files in line,
Containers swapped and metadata fine.
A tiny hop — the pipeline's bright.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'update onsite' is vague and generic, using non-descriptive language that doesn't clearly convey what specific aspect of the onsite module was updated. Replace with a more specific title such as 'Update onsite to pyonsite 0.0.3 and switch output format to idparquet' that clearly indicates the main changes.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codacy-production

codacy-production Bot commented Jun 13, 2026

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@modules/bigbio/onsite/meta.yml`:
- Around line 38-39: Update the meta.yml entry so the description matches the
pattern value: change the description field (currently "Protein/peptide
identifications file in idXML format") to refer to idparquet (e.g.,
"Protein/peptide identifications file in idparquet format") so the 'description'
and 'pattern' keys are consistent.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3a1bc770-27dc-43e8-b09a-dfec8cae2506

📥 Commits

Reviewing files that changed from the base of the PR and between dc3dcca and 84b9660.

📒 Files selected for processing (5)
  • modules/bigbio/onsite/environment.yml
  • modules/bigbio/onsite/main.nf
  • modules/bigbio/onsite/meta.yml
  • modules/bigbio/onsite/tests/main.nf.test
  • tests/config/test_data.config

Comment thread modules/bigbio/onsite/meta.yml Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/ci.yml:
- Around line 176-179: The download loop using curl in the for loop (iterating
over psms.parquet proteins.parquet protein_groups.parquet search_params.parquet
and using $DIR/$f and $BASE_URL/$f) can silently succeed on HTTP errors and hang
without timeouts; update the curl invocation to fail-fast and bounded by adding
-f (to treat HTTP errors as failures), -S for error output, explicit timeouts
(e.g. --connect-timeout and --max-time), and retry limits (e.g. --retry and
--retry-delay or --retry-connrefused), and ensure the loop exits on any curl
failure (e.g. by checking curl's exit code or enabling set -euo pipefail in the
script) so a bad download fails the CI immediately.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 91af71ba-05ff-4468-95c0-c355c185d90b

📥 Commits

Reviewing files that changed from the base of the PR and between c8f18f8 and f44bde1.

📒 Files selected for processing (2)
  • .github/workflows/ci.yml
  • tests/config/test_data.config
🚧 Files skipped from review as they are similar to previous changes (1)
  • tests/config/test_data.config

Comment thread .github/workflows/ci.yml
Comment on lines +176 to +179
for f in psms.parquet proteins.parquet protein_groups.parquet search_params.parquet; do
echo "Downloading $f ..."
curl -sL -o "$DIR/$f" "$BASE_URL/$f"
done

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Make the onsite fixture download fail-fast and bounded.

At Line 178, curl -sL can silently succeed on HTTP errors and has no timeout/retry controls. That can produce opaque nf-test failures later or flaky CI hangs.

Suggested hardening
       - name: Download onsite test data directory
         if: matrix.module == 'bigbio/onsite'
         run: |
+          set -euo pipefail
           DIR="tests/data/onsite/Phospho_redissolve_final_01_clean_perc.idparquet"
           mkdir -p "$DIR"
           BASE_URL="https://ftp.pride.ebi.ac.uk/pub/databases/pride/resources/proteomes/quantms-ci-github/onsite/Phospho_redissolve_final_01_clean_perc.idparquet"
           for f in psms.parquet proteins.parquet protein_groups.parquet search_params.parquet; do
             echo "Downloading $f ..."
-            curl -sL -o "$DIR/$f" "$BASE_URL/$f"
+            curl --fail --show-error --location \
+              --retry 3 --retry-all-errors --retry-delay 2 \
+              --connect-timeout 15 --max-time 120 \
+              -o "$DIR/$f" "$BASE_URL/$f"
+            test -s "$DIR/$f"
           done
           ls -lh "$DIR"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/ci.yml around lines 176 - 179, The download loop using
curl in the for loop (iterating over psms.parquet proteins.parquet
protein_groups.parquet search_params.parquet and using $DIR/$f and $BASE_URL/$f)
can silently succeed on HTTP errors and hang without timeouts; update the curl
invocation to fail-fast and bounded by adding -f (to treat HTTP errors as
failures), -S for error output, explicit timeouts (e.g. --connect-timeout and
--max-time), and retry limits (e.g. --retry and --retry-delay or
--retry-connrefused), and ensure the loop exits on any curl failure (e.g. by
checking curl's exit code or enabling set -euo pipefail in the script) so a bad
download fails the CI immediately.

@daichengxin daichengxin merged commit 18d45a9 into bigbio:main Jun 13, 2026
7 checks passed
@coderabbitai coderabbitai Bot mentioned this pull request Jun 22, 2026
11 tasks
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.

1 participant