Skip to content

feat: huggingface/download: support whole repository downloads - #13071

Merged
mwiewior merged 4 commits into
nf-core:masterfrom
mwiewior:huggingface-download-repository
Oct 2, 2026
Merged

mwiewior merged 4 commits into
nf-core:masterfrom
mwiewior:huggingface-download-repository

Conversation

@mwiewior

@mwiewior mwiewior commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

vepyr stores its prebuilt annotation caches on Hugging Face as public dataset repositories, for example biodatageeks/vepyr_116_GRCh38_ensembl. Each cache is a directory tree of Parquet shards and manifests. The existing huggingface/download module requires a single filename and emits that file; vepyr needs the cache directory to be downloaded and staged as a whole, optionally restricted to selected chromosomes. See the vepyr cache download documentation for the cache layout and Hugging Face download instructions.

This PR extends huggingface/download to provide that reusable directory-download capability. Its scope is confined to this module. Integration into vcf_annotate_vepyr will follow in #13070 after this dependency is merged.

Changes:

  • Accept [] or '' for hf_file to download a repository and emit its directory through the existing output channel. The directory name comes from ext.prefix, defaulting to meta.id.
  • Preserve the existing input tuple, output channel and behaviour for single-file downloads. Update stubs to support repository directories and nested file paths.
  • Support dataset selection, revision pinning and include/exclude filters through the existing ext.args mechanism. For vepyr caches, use --repo-type dataset; chromosome subsets must include the corresponding Parquet shards and every entity's chrom_manifest.json.
  • Refresh Docker/Singularity container metadata and AMD64/ARM64 Conda locks, retaining huggingface_hub=1.18.0. Supply the bundled CA certificates when SSL_CERT_FILE is unset so Xet downloads work in minimal containers, preserving TLS verification and explicit user overrides.
  • Add download regression tests and a native_arm64 test profile selecting the published ARM64 Docker image.

Validation:

  • All 10 module tests pass with both AMD64 Docker under emulation and native ARM64 Docker, using identical snapshots. Coverage includes single files, complete and filtered repositories, custom directory names, nested paths, stubs and an Xet-backed model file.
  • Both downstream llamacpppython/run CPU tests pass with their existing snapshots. Upstream AMD64 Docker, Singularity and Conda test jobs pass.
  • Local end-to-end verification downloads the published Ensembl 116 chr22 cache from Hugging Face, then runs bcftools/norm and vepyr/annotate. All 14 cache files match the Hub hashes; all 1,000 variants have CSQ annotations; the tabix index works; and the VCF body matches the existing subworkflow fixture exactly (MD5 1d4a92b815eb6193e6f4546f4ab35978). The cache is pinned to revision 15a048f0585a7a40d2d40cb1c2f7806638ee500b.
  • nf-core module lint and repository pre-commit hooks pass.

Generated by Codex

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 necessary, include test data in your PR.
  • Remove all TODO statements.
  • Broadcast software version numbers to topic: versions - See version_topics
  • Follow the naming conventions.
  • Follow the parameters requirements.
  • Follow the input/output options guidelines.
  • Add a resource label
  • Use BioConda and BioContainers if possible to fulfil software requirements.
  • Ensure that the test works with either Docker / Singularity. Conda CI tests can be quite flaky:
    • For modules:
      • nf-core modules test huggingface/download --profile docker (equivalent nf-test test ... --profile=+docker)
      • nf-core modules test huggingface/download --profile singularity (upstream CI)
      • nf-core modules test huggingface/download --profile conda (upstream CI)

Allow an empty hf_file to download and emit a repository directory. Cover dataset filters, prefixes, nested files and stubs while preserving existing single-file outputs. Refresh container metadata for current nf-core lint requirements.

Generated by Codex
Use the bundled certifi trust store when SSL_CERT_FILE is unset. This fixes Xet downloads in the refreshed Wave containers while retaining certificate verification and user-supplied CA bundles. Add a pinned Xet model repository regression test.

Generated by Codex
@mwiewior mwiewior changed the title huggingface/download: support whole repository downloads feat: huggingface/download: support whole repository downloads Oct 1, 2026
Select the published linux/arm64 container through test configuration, following nf-core architecture selection conventions. All ten tests pass on native ARM64 against the existing snapshots.

Generated by Codex
@mwiewior
mwiewior marked this pull request as ready for review October 1, 2026 10:48
@mwiewior
mwiewior requested a review from toniher October 1, 2026 10:53
@toniher
toniher requested a balanced review from Copilot October 1, 2026 14:34

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

The focused implementation preserves existing behaviour and has comprehensive regression coverage across the new download modes.

Review effort: Balanced
Findings: None

What changed in this PR

Extends huggingface/download to support complete or filtered repository downloads while preserving single-file behaviour.

Changes:

  • Adds directory downloads, nested paths, custom prefixes and CA bundle handling.
  • Adds regression and ARM64 profile coverage.
  • Refreshes multi-architecture container and Conda metadata.
File Description
main.nf Implements repository downloads and updated containers.
meta.yml Documents the expanded interface and container metadata.
tests/​main.nf.test Tests file, repository, filtered, Xet and stub modes.
tests/​main.nf.test.snap Records the expanded test results.
tests/​nextflow.config Configures arguments, prefixes and ARM64 testing.
.conda-lock/​linux_amd64-…txt Adds the AMD64 dependency lock.
.conda-lock/​linux_arm64-…txt Adds the ARM64 dependency lock.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@mwiewior
mwiewior enabled auto-merge October 1, 2026 19:12
…r test

Generated by Claude Opus 5.5

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@toniher

toniher commented Oct 2, 2026

Copy link
Copy Markdown
Member

Review follow-ups (pushed in 5f684dc):

  1. Removed the native_arm64 profile from tests/nextflow.config. It duplicated the shared arm64 profile in tests/config/nf-test.config, which CI already adds on ARM runners (,arm64,wave). It was also the only module test config in the repo with a profiles {} block, and it hard-coded the ARM64 image a second time. For a local ARM run, use nf-test test modules/nf-core/huggingface/download/tests --profile=+docker,arm64.
  2. The test config is now applied per test instead of once at the top. Following the testing spec, only the 5 tests that set params load ./nextflow.config. All 10 tests pass with the existing snapshots.
  3. output_path stays type: file in meta.yml. The channel emits either a file or a directory depending on hf_file, and keeping one channel preserves the existing API for downstream users such as llamacpppython/run. The description already covers the directory case.
  4. Documented the SSL_CERT_FILE behaviour in the meta.yml description. When the variable is unset, the module uses certifi's CA bundle so that Xet downloads work in minimal containers. Anyone behind a TLS-intercepting proxy, or with a custom CA, should export SSL_CERT_FILE.

Comment added by Claude Opus 5.5

@toniher toniher left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks Good! Provided minor modifications per existing recommendations

@mwiewior
mwiewior added this pull request to the merge queue Oct 2, 2026
Merged via the queue into nf-core:master with commit e06e280 Oct 2, 2026
119 checks passed
@mwiewior
mwiewior deleted the huggingface-download-repository branch October 2, 2026 10:19
mwiewior added a commit to mwiewior/modules that referenced this pull request Oct 2, 2026
Load ./nextflow.config only in the tests that rely on its ext.args
(BCFTOOLS_NORM, HUGGINGFACE_DOWNLOAD), following the nf-core testing
spec and the review of nf-core#13071. The no-normalise test runs without it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@mwiewior mwiewior mentioned this pull request Oct 2, 2026
12 of 14 tasks
famosab pushed a commit to famosab/modules that referenced this pull request Oct 5, 2026
* Add vcf_annotate_vepyr subworkflow

Compose optional bcftools normalisation with vepyr annotation and add published-fixture tests for indexed, unindexed, bypass and stub paths.

Generated by Codex

* vcf_annotate_vepyr: support Hugging Face cache repositories

Add an optional repository input that downloads one shared cache before annotation, retaining the existing local-cache path. Document revision and contig filters. Exercise local-cache bypass, remote stubs and a real two-sample annotation using the published chr22 cache.

Generated by Codex

* vcf_annotate_vepyr: apply test config per test

Load ./nextflow.config only in the tests that rely on its ext.args
(BCFTOOLS_NORM, HUGGINGFACE_DOWNLOAD), following the nf-core testing
spec and the review of nf-core#13071. The no-normalise test runs without it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* vcf_annotate_vepyr: run UNTAR setup only where the cache tarball is used

Move the UNTAR setup into the three local-cache tests so the Hugging
Face and stub tests no longer fetch and extract cache.tar.gz. Mention
the optional Hugging Face download in the meta.yml summary.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* bcftools/norm: refuse to write output over the staged input

With no ext.prefix and meta.id equal to the input basename, the default
output name matches the staged input symlink, so bcftools norm rewrites
the source file through it. Add the input/output collision guard used
by bcftools/annotate, csq, filter, convert and pluginfilltags, in script
and stub, with module and vcf_annotate_vepyr regression tests.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* vcf_annotate_vepyr: move the bcftools/norm guard to nf-core#13076

The input/output collision guard is a module fix, so it now lives in its
own PR. Keep the subworkflow regression test, which passes once nf-core#13076
is merged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* vcf_annotate_vepyr: drop the collision test until nf-core#13076 lands

The test asserts the bcftools/norm guard from nf-core#13076, which is not on
master yet, so it fails in CI. Restore the meta.yml wording that matches
the current module; both come back once nf-core#13076 is merged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* vcf_annotate_vepyr: rely on the bcftools/norm default prefix from nf-core#13076

bcftools/norm now defaults to ${meta.id}_norm and refuses colliding
prefixes, so drop the ext.prefix workaround from the documented config.
Add a test where the sample ID matches the input basename.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* vcf_annotate_vepyr: address review of nf-core#13070

- Prefix val_hf_repo with datasets/, so HUGGINGFACE_DOWNLOAD needs no
  required --repo-type in ext.args; only --revision/--include remain.
- Fail when both ch_cache and val_hf_repo are set, or neither, and test
  both failures.
- Test normalisation on input_multiallelic.vcf.gz (986 records, 14
  multiallelic; nf-core/test-datasets#2298): the output has 1,000
  biallelic records and the same variantsMD5 as annotating the
  pre-split input.vcf.gz without normalisation.
- Drop the basename test (covered by the bcftools/norm module tests)
  and use channel instead of Channel.
- meta.yml: reword the bcftools norm configuration, drop the ext.prefix
  paragraph, keep one "default to true", document that ch_cache and
  ch_fasta must be value channels and that BCF or plain VCF output
  makes VEPYR_ANNOTATE run a single pipeline.

Generated by Claude Code

* vcf_annotate_vepyr: build the multiallelic test input in setup

Instead of a new test-datasets file (nf-core/test-datasets#2298), the
normalise tests join input.vcf.gz back into multiallelic records with
an aliased BCFTOOLS_NORM in setup: --multiallelics +both turns 1,000
records into 986, 14 of them multiallelic, and the subworkflow splits
them again. --do-not-normalize is needed because the module always
passes --fasta-ref, and left-aligning fails on chr22 vs 22. Snapshots
are unchanged.

Generated by Claude Code

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants