Repository navigation
Fix InterProScan ignoring staged database - #115
Merged
Merged
Conversation
Update the interproscan module to pick up nf-core/modules#13009, so the staged database is actually used. Default interproscan_db_url to the 5.59-91.0 release matching the container, pass the inner data/ folder of an untarred release to INTERPROSCAN, and drop Hamap from test profiles (test tarball ships hamap/2023_05, 5.59 expects 2021_04). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
erikrikarddaniel
approved these changes
Sep 26, 2026
erikrikarddaniel
left a comment
erikrikarddaniel
left a comment
Member
There was a problem hiding this comment.
Reviewed with Claude Code.
Checked the mechanism rather than re-deriving it:
modules/nf-core/interproscan/{main.nf,meta.yml,tests/nextflow_database.config}are byte-identical to nf-core/modules at3137cddd, so this is a cleannf-core modules updateand not a hand-patch.- The
data/resolution is right for both layouts.UNTARstrips a leading component only when the archive has a single top-level entry, so the 5.59-91.0 release (interproscan-5.59-91.0/) untars tobin/ data/ setup.py ...and the newmappicksdata/, while the test tarball (single top-leveldata/) untars tohamap/ sfld/ tigrfam/and passes through unchanged. Both end up staged asdata, matchingdata.directory=$PWD/datain the module. - The 5.59-91.0 tarball is 6,594,330,558 bytes, so the 6.6 GB in
docs/usage.mdis right. - The test tarball ships
hamap/2023_05andtigrfam/15.0, which matches the reasoning for droppingHamapfrom the test profiles.
Four non-blocking points, all about what happens after this merges rather than about the fix. Details inline.
- Neither CI path exercises the new
data/branch — the test tarball takes the pass-through side, so the exact layout #114 reported has no test. - Dropping
Hamapfrom the test profiles leaves the defaultinterproscan_applicationsuntested, and thehamap/2023_05files in the test data unused. - Nothing ties
interproscan_db_urlto the module's container version, so the next module update that bumps InterProScan re-creates this bug silently. PIRSFstays in the default applications, so cause 3 of #114 is still live on a default multi-sample run, andhmmpresswriting intoUNTAR's output also breaks-resumefor that task.
Approving — none of this blocks.
| // Functional annotation | ||
| skip_interproscan = false | ||
| interproscan_db_url = "https://ftp.ebi.ac.uk/pub/software/unix/iprscan/5/5.72-103.0/interproscan-5.72-103.0-64-bit.tar.gz" | ||
| interproscan_db_url = "https://ftp.ebi.ac.uk/pub/software/unix/iprscan/5/5.59-91.0/interproscan-5.59-91.0-64-bit.tar.gz" |
Member
There was a problem hiding this comment.
This default now has to track the InterProScan version in the module's container, and nothing enforces that. The next nf-core modules update that bumps the container re-creates #114 silently, with the same "database not found" failure that only shows up on a real run. A note at the declaration is the cheap guard:
Suggested change
| interproscan_db_url = "https://ftp.ebi.ac.uk/pub/software/unix/iprscan/5/5.59-91.0/interproscan-5.59-91.0-64-bit.tar.gz" | |
| // must match the interproscan module's container version: newer releases rename member databases | |
| interproscan_db_url = "https://ftp.ebi.ac.uk/pub/software/unix/iprscan/5/5.59-91.0/interproscan-5.59-91.0-64-bit.tar.gz" |
The same coupling applies to interproscan_applications two lines down, since TIGRFAM only exists up to 5.59.
Collaborator
Author
There was a problem hiding this comment.
Sadly (luckily?) there are no more interproscan bioconda (+containers) updates for the foreseeable future. Interproscan has become its own Nextflow pipeline now (non nf-core). Updating the nf-core module would mean completely moving the underlying software, and would require NExtflow packaging into biconda which is kinda unusual (for now).
|
|
||
| // A full release untars to the whole install; INTERPROSCAN needs its inner data/ folder | ||
| ch_interproscan_db = ch_interproscan_db | ||
| .map { database -> database.resolve('data').isDirectory() ? database.resolve('data') : database } |
Member
There was a problem hiding this comment.
The branch this adds is never taken in CI, so the layout #114 reported has no test.
UNTAR strips a leading component only when the archive has a single top-level entry. interproscan_test.tar.gz has exactly one, data/, so it untars to hamap/ sfld/ tigrfam/ and resolve('data') is not a directory — both the pipeline test and the subworkflow test take the pass-through side. The real 5.59-91.0 release has interproscan-5.59-91.0/ as its single top-level entry and untars to bin/ data/ setup.py ..., which is the only case where the new map does anything.
Repacking the test tarball with a wrapper directory (interproscan-test/data/..., plus a token interproscan.properties) would make the untarred layout match a real release and give this line a test. Worth a follow-up on the test-datasets branch rather than something to hold this PR for.
Collaborator
Author
There was a problem hiding this comment.
Thanks. Will exercise this in case the module starts misbehaving another time
| // Functional annotation | ||
| interproscan_db_url = params.pipelines_testdata_base_path + 'proteinannotator/testdata/interproscan/interproscan_test.tar.gz' | ||
| interproscan_applications = 'Hamap,TIGRFAM,sfld' | ||
| interproscan_applications = 'TIGRFAM,sfld' |
Member
There was a problem hiding this comment.
With Hamap dropped here and in test_full.config, the default interproscan_applications (Hamap,PANTHER,PIRSF,TIGRFAM,sfld) is no longer covered by any test — and the hamap/2023_05 files in the test tarball are now dead weight. Regenerating the test data with hamap/2021_04 instead would let the test profiles keep using Hamap and stay closer to what a default run does. Same follow-up as the wrapper-directory point on the subworkflow.
Collaborator
Author
There was a problem hiding this comment.
Agreed, the default set has no coverage now. Regenerating the test data with hamap/2021_04 (together with the wrapper-directory repack from the subworkflow thread) is a test-datasets change, so I will leave it for a follow-up rather than this bugfix release.
| ``` | ||
|
|
||
| > [!NOTE] | ||
| > The HMMs in the 5.59-91.0 release are not pre-indexed. The default applications do not need them indexed, except PIRSF, which indexes its own HMMs the first time it runs. When several samples run InterProScan in parallel against a fresh database, they may all try to index the same PIRSF files at once. To avoid this, index the database once before running the pipeline, from inside the untarred release directory (the one containing `setup.py` and `interproscan.properties`, not `data/`): |
Member
There was a problem hiding this comment.
Is leaving PIRSF in the default interproscan_applications deliberate? As it stands, cause 3 in #114 is still live on the default path: a user who runs the pipeline with more than one sample and no pre-existing database gets parallel tasks pressing the same PIRSF HMMs, and the documented fix only applies on a second run via --interproscan_db.
There is a second effect worth naming: the module stages the database as a symlink, so hmmpress writes into UNTAR_INTERPROSCAN's own output directory. That mutates an upstream task's output, which breaks -resume for UNTAR and is the same class of bug as nf-core/modules#12799, independently of the race.
Pressing once in a small local step after UNTAR would close both, and would also make the default download path work unattended. Dropping PIRSF from the default list would be the smaller change. Either is fine by me — just flagging that the default configuration is still the one that fails.
Collaborator
Author
There was a problem hiding this comment.
Not deliberate, so I dropped it: PIRSF is out of the default interproscan_applications in aa70392 (config, schema, docs, CHANGELOG). The usage note now explains that PIRSF is opt-in and shows how to pre-index the database before adding it back. That removes the race on the default path, and since the default applications never run hmmpress, nothing writes into UNTAR_INTERPROSCAN's output by default either. A one-off local indexing step after UNTAR would let PIRSF go back into the defaults; I will consider it after this release.
On a fresh pipeline-downloaded database, parallel INTERPROSCAN tasks race to hmmpress the same PIRSF HMMs. Keep it opt-in; usage docs explain how to pre-index and re-enable it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Sep 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #114.
Changes
interproscanmodule to pick up the fix for interproscan: staged database is ignored (relative INTERPROSCAN_CONF, wrong bin/data directories) modules#13009. The module now setsdata.directoryto the stageddata/and uses an absoluteINTERPROSCAN_CONF, so the database passed to the pipeline is actually used instead of the container's sample data.interproscan_db_urlto the 5.59-91.0 release, matching the module's container (bioconda stopped at 5.59). The 5.72-103.0 release has different member database paths (e.g.panther/19.0,hamap/2023_05, TIGRFAM replaced by NCBIfam) that 5.59 cannot find.FUNCTIONAL_ANNOTATION: pass the innerdata/directory toINTERPROSCANwhen the database directory contains one. Applies to both the downloaded database and--interproscan_db, so either the untarred release directory or itsdata/can be supplied.TIGRFAM,sfldinstead ofHamap,TIGRFAM,sfld, matching the module test. The test tarball shipshamap/2023_05, while 5.59 expects2021_04.docs/usage.mdand schema help: document the 5.59-91.0 requirement, the release layout, and where to runpython3 setup.py -f interproscan.propertiesto pre-index the HMMs. The default applications do not need indexed HMMs except PIRSF, which indexes its own on first run; pre-indexing avoids parallel tasks doing that at the same time.Testing
subworkflows/local/functional_annotationnf-test and pipeline-profile testnf-test pass (singularity), snapshots unchanged.data.directorypoints to the stageddata/.--interproscan_dbpointing at a release-style<dir>/data/...layout resolves and uses<dir>/data.data/succeeded earlier via a bind mount workaround.PR checklist
nf-core pipelines lint).nextflow run . -profile test,singularity --outdir <OUTDIR>).docs/usage.mdis updated.CHANGELOG.mdis updated.🤖 Generated with Claude Code