Skip to content

Commit 64b1d2f

Browse files
druvusclaude
andcommitted
fix: enforce --offline flag, add datasets CLI check, and log missing GTDB index
The --offline flag was parsed but never enforced during simulation. Now SpeciesResolver and download_genome respect offline mode by skipping NCBI network calls and raising clear errors for uncached genomes. Also adds an early check for the datasets CLI before attempting downloads, with actionable install instructions. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 609c070 commit 64b1d2f

4 files changed

Lines changed: 133 additions & 7 deletions

File tree

nanopore_simulator/core/simulator.py

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ def _resolve_species_inputs(self) -> None:
162162
):
163163
return
164164

165-
resolver = SpeciesResolver()
165+
resolver = SpeciesResolver(offline=self.config.offline_mode)
166166
resolved_genomes: List[Path] = []
167167
abundances: List[float] = []
168168

@@ -192,7 +192,9 @@ def _resolve_species_inputs(self) -> None:
192192
if ref is None:
193193
raise ValueError(f"Could not resolve organism: {org.name}")
194194

195-
genome_path = download_genome(ref, resolver.cache)
195+
genome_path = download_genome(
196+
ref, resolver.cache, offline=self.config.offline_mode
197+
)
196198
resolved_genomes.append(genome_path)
197199
abundances.append(org.abundance)
198200

@@ -208,7 +210,9 @@ def _resolve_species_inputs(self) -> None:
208210
msg += f". Did you mean: {', '.join(suggestions)}?"
209211
raise ValueError(msg)
210212

211-
genome_path = download_genome(ref, resolver.cache)
213+
genome_path = download_genome(
214+
ref, resolver.cache, offline=self.config.offline_mode
215+
)
212216
resolved_genomes.append(genome_path)
213217

214218
# Resolve taxids
@@ -217,7 +221,9 @@ def _resolve_species_inputs(self) -> None:
217221
if ref is None:
218222
raise ValueError(f"Could not resolve taxid: {taxid}")
219223

220-
genome_path = download_genome(ref, resolver.cache)
224+
genome_path = download_genome(
225+
ref, resolver.cache, offline=self.config.offline_mode
226+
)
221227
resolved_genomes.append(genome_path)
222228

223229
# Set abundances

nanopore_simulator/core/species.py

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,10 @@ def __init__(self, index_path: Path) -> None:
126126
def _load_index(self) -> None:
127127
"""Load the TSV index file into memory."""
128128
if not self.index_path.exists():
129+
logger.debug(
130+
"GTDB index not found at %s; species resolution will use NCBI.",
131+
self.index_path,
132+
)
129133
return
130134
with open(self.index_path) as f:
131135
# Skip header line
@@ -314,6 +318,7 @@ def __init__(
314318
self,
315319
index_dir: Optional[Path] = None,
316320
cache_dir: Optional[Path] = None,
321+
offline: bool = False,
317322
) -> None:
318323
"""Initialize the species resolver.
319324
@@ -322,6 +327,8 @@ def __init__(
322327
~/.nanorunner/indexes/ using the HOME environment variable.
323328
cache_dir: Directory for genome cache. If None, defaults to
324329
~/.nanorunner/genomes/ using the HOME environment variable.
330+
offline: If True, skip NCBI network lookups and rely only on the
331+
local GTDB index and cached genomes.
325332
"""
326333
if index_dir is None:
327334
home = Path(os.environ.get("HOME", Path.home()))
@@ -330,6 +337,7 @@ def __init__(
330337
self._gtdb = GTDBIndex(index_dir / "gtdb_species.tsv")
331338
self._ncbi = NCBIResolver()
332339
self._cache = GenomeCache(cache_dir)
340+
self._offline = offline
333341

334342
def resolve(self, species_name: str) -> Optional[GenomeRef]:
335343
"""Resolve a species name to a genome reference.
@@ -349,7 +357,9 @@ def resolve(self, species_name: str) -> Optional[GenomeRef]:
349357
if ref is not None:
350358
return ref
351359

352-
# Fall back to NCBI
360+
# Fall back to NCBI (skip in offline mode)
361+
if self._offline:
362+
return None
353363
return self._ncbi.resolve_by_name(species_name)
354364

355365
def resolve_taxid(self, taxid: int) -> Optional[GenomeRef]:
@@ -361,6 +371,8 @@ def resolve_taxid(self, taxid: int) -> Optional[GenomeRef]:
361371
Returns:
362372
GenomeRef if found, None otherwise.
363373
"""
374+
if self._offline:
375+
return None
364376
return self._ncbi.resolve_by_taxid(taxid)
365377

366378
def suggest(self, partial_name: str) -> List[str]:
@@ -384,7 +396,9 @@ def cache(self) -> GenomeCache:
384396
return self._cache
385397

386398

387-
def download_genome(ref: GenomeRef, cache: GenomeCache) -> Path:
399+
def download_genome(
400+
ref: GenomeRef, cache: GenomeCache, offline: bool = False
401+
) -> Path:
388402
"""Download a genome and cache it.
389403
390404
Uses the NCBI datasets CLI to download the genome sequence for the
@@ -394,19 +408,39 @@ def download_genome(ref: GenomeRef, cache: GenomeCache) -> Path:
394408
Args:
395409
ref: Genome reference specifying the accession to download.
396410
cache: Genome cache instance for storing the downloaded genome.
411+
offline: If True, raise an error instead of downloading when the
412+
genome is not already cached.
397413
398414
Returns:
399415
Path to the cached genome file (gzip compressed).
400416
401417
Raises:
402-
RuntimeError: If the download fails or no .fna file is found.
418+
RuntimeError: If the download fails, no .fna file is found,
419+
offline mode is enabled and the genome is not cached, or
420+
the datasets CLI is not installed.
403421
"""
404422
# Check cache first
405423
cached_path = cache.get_cached_path(ref)
406424
if cached_path.exists():
407425
logger.info(f"Using cached genome: {cached_path}")
408426
return cached_path
409427

428+
# Offline mode: genome must already be cached
429+
if offline:
430+
raise RuntimeError(
431+
f"Genome {ref.accession} ({ref.name}) is not cached and "
432+
"offline mode is enabled. Run 'nanorunner download' first "
433+
"to cache the required genomes."
434+
)
435+
436+
# Check that datasets CLI is available before attempting download
437+
if shutil.which("datasets") is None:
438+
raise RuntimeError(
439+
f"Cannot download genome {ref.accession}: "
440+
"the 'datasets' CLI (ncbi-datasets-cli) is not installed. "
441+
"Install with: conda install -c conda-forge ncbi-datasets-cli"
442+
)
443+
410444
# Download via datasets CLI
411445
logger.info(f"Downloading genome: {ref.accession}")
412446

tests/test_simulator_species.py

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -325,3 +325,39 @@ def test_no_resolution_for_copy_operation(self, tmp_path):
325325
sim = NanoporeSimulator(config, enable_monitoring=False)
326326
# Should not have instantiated resolver
327327
mock_resolver_cls.assert_not_called()
328+
329+
def test_offline_mode_passes_flag(self, tmp_path):
330+
"""Test that offline_mode is forwarded to resolver and download."""
331+
config = SimulationConfig(
332+
target_dir=tmp_path / "output",
333+
operation="generate",
334+
species_inputs=["Escherichia coli"],
335+
sample_type="pure",
336+
read_count=10,
337+
reads_per_file=10,
338+
offline_mode=True,
339+
)
340+
341+
with patch(
342+
"nanopore_simulator.core.simulator.SpeciesResolver"
343+
) as mock_resolver_cls:
344+
mock_resolver = MagicMock()
345+
mock_resolver_cls.return_value = mock_resolver
346+
347+
mock_ref = GenomeRef(
348+
"Escherichia coli", "GCF_000005845.2", "gtdb", "bacteria"
349+
)
350+
mock_resolver.resolve.return_value = mock_ref
351+
352+
genome_path = tmp_path / "genome.fa"
353+
genome_path.write_text(">chr1\nATCGATCGATCG\n")
354+
355+
with patch(
356+
"nanopore_simulator.core.simulator.download_genome",
357+
return_value=genome_path,
358+
) as mock_download:
359+
sim = NanoporeSimulator(config, enable_monitoring=False)
360+
mock_resolver_cls.assert_called_once_with(offline=True)
361+
mock_download.assert_called_once_with(
362+
mock_ref, mock_resolver.cache, offline=True
363+
)

tests/test_species.py

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,12 @@ def test_lookup_case_insensitive(self, tmp_path):
155155
assert ref is not None
156156
assert ref.name == "Escherichia coli"
157157

158+
def test_missing_index_file(self, tmp_path):
159+
"""Test GTDBIndex with nonexistent file returns None without error."""
160+
index = GTDBIndex(tmp_path / "nonexistent.tsv")
161+
ref = index.lookup("Escherichia coli")
162+
assert ref is None
163+
158164
def test_fuzzy_suggestions(self, tmp_path):
159165
"""Test suggest returns matching species names for partial input"""
160166
index_file = tmp_path / "gtdb_species.tsv"
@@ -306,6 +312,20 @@ def test_cache_property(self, tmp_path, monkeypatch):
306312
assert isinstance(resolver.cache, GenomeCache)
307313
assert resolver.cache.cache_dir == tmp_path / "genomes"
308314

315+
def test_resolve_offline_skips_ncbi(self, tmp_path, monkeypatch):
316+
"""Test that offline mode skips NCBI resolution."""
317+
# Empty GTDB index so lookup will miss
318+
index_file = tmp_path / "indexes" / "gtdb_species.tsv"
319+
index_file.parent.mkdir(parents=True, exist_ok=True)
320+
index_file.write_text("species\taccession\tdomain\n")
321+
monkeypatch.setenv("HOME", str(tmp_path))
322+
323+
resolver = SpeciesResolver(index_dir=tmp_path / "indexes", offline=True)
324+
with patch.object(resolver._ncbi, "resolve_by_name") as mock_ncbi:
325+
ref = resolver.resolve("Saccharomyces cerevisiae")
326+
assert ref is None
327+
mock_ncbi.assert_not_called()
328+
309329
def test_default_index_dir(self, tmp_path, monkeypatch):
310330
"""Test that default index directory is used when not specified"""
311331
# Create default index location
@@ -433,3 +453,33 @@ def create_empty_zip(*args, **kwargs):
433453

434454
with pytest.raises(RuntimeError, match="No .fna file found"):
435455
download_genome(ref, cache)
456+
457+
def test_download_genome_no_datasets_cli(self, tmp_path):
458+
"""Test error when datasets CLI is not installed."""
459+
cache = GenomeCache(cache_dir=tmp_path)
460+
ref = GenomeRef("E. coli", "GCF_000005845.2", "gtdb", "bacteria")
461+
462+
with patch("nanopore_simulator.core.species.shutil.which", return_value=None):
463+
with pytest.raises(RuntimeError, match="ncbi-datasets-cli"):
464+
download_genome(ref, cache)
465+
466+
def test_download_genome_offline_not_cached(self, tmp_path):
467+
"""Test error when offline mode is enabled and genome is not cached."""
468+
cache = GenomeCache(cache_dir=tmp_path)
469+
ref = GenomeRef("E. coli", "GCF_000005845.2", "gtdb", "bacteria")
470+
471+
with pytest.raises(RuntimeError, match="offline mode"):
472+
download_genome(ref, cache, offline=True)
473+
474+
def test_download_genome_offline_cached(self, tmp_path):
475+
"""Test that offline mode returns cached genome without error."""
476+
cache = GenomeCache(cache_dir=tmp_path)
477+
ref = GenomeRef("E. coli", "GCF_000005845.2", "gtdb", "bacteria")
478+
479+
# Pre-create cached file
480+
cached_path = cache.get_cached_path(ref)
481+
cached_path.parent.mkdir(parents=True, exist_ok=True)
482+
cached_path.write_text("cached")
483+
484+
path = download_genome(ref, cache, offline=True)
485+
assert path == cached_path

0 commit comments

Comments
 (0)