Repository navigation
Fix bambu requiring internet access when given a local FASTA genome path - #588
Merged
Merged
Conversation
checkInputSequence() unconditionally called BSgenome::available.genomes() for any character genome argument, including local FASTA file paths - the documented, common case. That call reaches out to Bioconductor's package repository over the network, so bambu failed on offline clusters/containers even when the user supplied a local file (issue #339). Now the BSgenome lookup is only attempted when the string is not an existing local file, and is wrapped in tryCatch so a blocked network call falls through to the existing FASTA-read error path instead of crashing with a raw Bioconductor/BiocManager error. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
cying111
approved these changes
Sep 2, 2026
cying111
left a comment
Collaborator
There was a problem hiding this comment.
the change looks good to me and I have tested it offline where local fasta files are loaded, before the change there is an error similar like the one described in the issue, after the change it can run smoothly
Collaborator
|
same as community issue #581, can inform the user once it's approved. |
lingminhao
approved these changes
Oct 1, 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.
Summary
bambu()failed on offline machines (e.g. an offline HPC cluster, or a self-contained biocontainers image with no internet egress — see comment) even when given a plain local FASTA file path forgenome.checkInputSequence()inR/bambu_utilityFunctions.Runconditionally calledBSgenome::available.genomes()for any charactergenomeargument, before ever checking whether the string was simply a local file path. That call reaches out to Bioconductor's package repository over the network, so it threw a raw Bioconductor/BiocManager error instead of just reading the FASTA.BSgenome::available.genomes()/getBSgenome()lookup when the string is not an existing local file, and wrap that lookup intryCatchso a blocked/failed network call falls through to the existing FASTA-read path (and its existing clear error message) instead of crashing.Test plan
devtools::load_all()+ tracedBSgenome::available.genomes()to confirm it is no longer called whencheckInputSequence()is given the package's local test FASTA (inst/extdata/Homo_sapiens.GRCh38.dna_sm.primary_assembly_chr9_1_1000000.fa)"Input genome file not readable."errortestthat::test_file()passing locally fortest_bambu_arguments.R,test_scoreRC.R,test_isore.R(all exercisecheckInputSequence()/bambu()with a local FASTA path)test_setNDR.Rfails identically before and after this change (missing fixture path in this environment) — confirmed unrelated to this fix🤖 Generated with Claude Code