Skip to content

Commit 2f9c0fe

Browse files
committed
add explicit docker_image support across CLI, pipeline, and job layer
1 parent 92cd996 commit 2f9c0fe

7 files changed

Lines changed: 240 additions & 7 deletions

File tree

tests/test_cli.py

Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
import pytest
2+
3+
from waters2mzml.cli import convert
4+
5+
6+
def test_docker_without_image_errors(tmp_path):
7+
raw = tmp_path / "raw"
8+
raw.mkdir()
9+
out = tmp_path / "mzml"
10+
out.mkdir()
11+
12+
with pytest.raises(Exception) as exc:
13+
convert(
14+
input=raw,
15+
output=out,
16+
centroid=False,
17+
base_dir=tmp_path,
18+
parallel=1,
19+
docker=True,
20+
docker_image=None,
21+
retries=0,
22+
log_level="INFO",
23+
)
24+
25+
assert "docker-image" in str(exc.value).lower()
26+
27+
28+
def test_docker_with_image_passes_validation(tmp_path, monkeypatch):
29+
called = {}
30+
31+
def fake_pipeline(**kwargs):
32+
called.update(kwargs)
33+
34+
monkeypatch.setattr("waters2mzml.cli.run_pipeline", fake_pipeline)
35+
36+
raw = tmp_path / "raw"
37+
raw.mkdir()
38+
(raw / "sample.raw").mkdir()
39+
40+
out = tmp_path / "mzml"
41+
out.mkdir()
42+
43+
convert(
44+
input=raw,
45+
output=out,
46+
centroid=False,
47+
base_dir=tmp_path,
48+
parallel=1,
49+
docker=True,
50+
docker_image="my/image",
51+
retries=0,
52+
log_level="INFO",
53+
)
54+
55+
assert called["use_docker"] is True
56+
assert called["docker_image"] == "my/image"
57+
58+
59+
def test_parallel_pipeline_receives_docker_image(tmp_path, monkeypatch):
60+
called = {}
61+
62+
def fake_parallel(**kwargs):
63+
called.update(kwargs)
64+
65+
monkeypatch.setattr("waters2mzml.cli.run_pipeline_parallel", fake_parallel)
66+
67+
raw = tmp_path / "raw"
68+
raw.mkdir()
69+
(raw / "sample.raw").mkdir()
70+
71+
out = tmp_path / "mzml"
72+
out.mkdir()
73+
74+
convert(
75+
input=raw,
76+
output=out,
77+
centroid=False,
78+
base_dir=tmp_path,
79+
parallel=4,
80+
docker=True,
81+
docker_image="pwiz/msconvert",
82+
retries=0,
83+
log_level="INFO",
84+
)
85+
86+
assert called["use_docker"] is True
87+
assert called["docker_image"] == "pwiz/msconvert"
88+
assert called["jobs"] == 4
89+
90+
91+
def test_native_mode_does_not_require_image(tmp_path, monkeypatch):
92+
called = {}
93+
94+
def fake_pipeline(**kwargs):
95+
called.update(kwargs)
96+
97+
monkeypatch.setattr("waters2mzml.cli.run_pipeline", fake_pipeline)
98+
99+
raw = tmp_path / "raw"
100+
raw.mkdir()
101+
(raw / "sample.raw").mkdir()
102+
103+
out = tmp_path / "mzml"
104+
out.mkdir()
105+
106+
convert(
107+
input=raw,
108+
output=out,
109+
centroid=False,
110+
base_dir=tmp_path,
111+
parallel=1,
112+
docker=False,
113+
docker_image=None,
114+
retries=0,
115+
log_level="INFO",
116+
)
117+
118+
assert called["use_docker"] is False
119+
assert called["docker_image"] is None
120+
121+
122+
def test_invalid_paths_fail_cleanly(tmp_path, caplog):
123+
out = tmp_path / "mzml"
124+
out.mkdir()
125+
126+
convert(
127+
input=tmp_path / "does_not_exist",
128+
output=out,
129+
centroid=False,
130+
base_dir=tmp_path,
131+
parallel=1,
132+
docker=False,
133+
docker_image=None,
134+
retries=0,
135+
log_level="INFO",
136+
)
137+
138+
# The pipeline should NOT raise — it should log a message
139+
assert "no .raw folders found" in caplog.text.lower()

tests/test_msconvert.py

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,3 +235,45 @@ def fake_build(self):
235235
args = cfg.build_msconvert_args()
236236
assert "--foo" in args
237237
assert "--bar=123" in args
238+
239+
240+
def test_docker_image_required(tmp_path):
241+
raw = tmp_path / "sample.raw"
242+
raw.mkdir()
243+
244+
cfg = ConversionConfig(centroid=False, use_docker=True, docker_image=None)
245+
246+
with pytest.raises(MsconvertError):
247+
_run_msconvert_docker(raw, cfg)
248+
249+
250+
def test_docker_command_construction(monkeypatch, tmp_path):
251+
raw = tmp_path / "sample.raw"
252+
raw.mkdir()
253+
254+
cfg = ConversionConfig(
255+
centroid=True,
256+
use_docker=True,
257+
docker_image="my/image",
258+
)
259+
260+
captured = {}
261+
262+
def fake_run(cmd, capture_output, text):
263+
captured["cmd"] = cmd
264+
265+
class P:
266+
returncode = 0
267+
stderr = ""
268+
269+
return P()
270+
271+
monkeypatch.setattr(subprocess, "run", fake_run)
272+
273+
_run_msconvert_docker(raw, cfg)
274+
275+
cmd = captured["cmd"]
276+
assert cmd[0] == "docker"
277+
assert "my/image" in cmd
278+
assert "/data/sample.raw" in cmd
279+
assert "--outdir" in cmd

tests/test_pipeline_integration.py

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,3 +69,40 @@ def test_full_pipeline(tmp_path, monkeypatch):
6969
assert "scan=1" in text
7070
assert "scan=2" in text
7171
assert 'value="2"' in text
72+
73+
74+
def test_pipeline_passes_docker_image(monkeypatch, tmp_path):
75+
called = {}
76+
77+
def fake_process(**kwargs):
78+
called.update(kwargs)
79+
80+
class R:
81+
success = True
82+
warnings = []
83+
qc = None
84+
mzml_path = tmp_path / "x.mzML"
85+
raw_dir = tmp_path / "raw/sample.raw"
86+
error = None
87+
88+
return R()
89+
90+
monkeypatch.setattr("waters2mzml.pipeline.process_single_raw", fake_process)
91+
92+
raw = tmp_path / "raw"
93+
raw.mkdir()
94+
(raw / "sample.raw").mkdir()
95+
96+
out = tmp_path / "mzml"
97+
98+
run_pipeline(
99+
base_dir=tmp_path,
100+
input_dir=raw,
101+
output_dir=out,
102+
centroid=False,
103+
use_docker=True,
104+
docker_image="img",
105+
)
106+
107+
assert called["use_docker"] is True
108+
assert called["docker_image"] == "img"

waters2mzml/cli.py

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,12 @@ def convert(
5656
docker: bool = typer.Option(
5757
False,
5858
"--docker",
59-
help="Run msconvert inside a Docker container (requires a user-provided image)",
59+
help="Run msconvert inside a Docker container",
60+
),
61+
docker_image: str | None = typer.Option(
62+
None,
63+
"--docker-image",
64+
help="Docker image containing msconvert (required with --docker)",
6065
),
6166
retries: int = typer.Option(
6267
0,
@@ -76,6 +81,9 @@ def convert(
7681
"""
7782
setup_logging(log_level)
7883

84+
if docker and not docker_image:
85+
raise typer.BadParameter("You must specify --docker-image when using --docker")
86+
7987
if parallel <= 1:
8088
# Sequential pipeline
8189
run_pipeline(
@@ -84,6 +92,7 @@ def convert(
8492
output_dir=output,
8593
centroid=centroid,
8694
use_docker=docker,
95+
docker_image=docker_image,
8796
)
8897
else:
8998
# Parallel pipeline
@@ -95,6 +104,7 @@ def convert(
95104
jobs=parallel,
96105
use_docker=docker,
97106
retries=retries,
107+
docker_image=docker_image,
98108
)
99109

100110

waters2mzml/config.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ class ConversionConfig:
88
ms_level_filter: str = "1-2"
99
compression: str = "--zlib --32"
1010
use_docker: bool = False
11-
docker_image: str = "proteowizard/msconvert:latest"
11+
docker_image: str | None = None
1212

1313
def build_msconvert_args(self) -> str:
1414
# Mirrors original config strings

waters2mzml/job.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ def process_single_raw(
5858
output_dir: Path,
5959
centroid: bool,
6060
use_docker: bool = False,
61+
docker_image: str | None = None,
6162
do_postprocess: bool = True,
6263
) -> JobResult:
6364
"""
@@ -77,7 +78,11 @@ def process_single_raw(
7778

7879
# 2) Convert with msconvert
7980
logger.debug(f"Converting {raw_dir} with msconvert")
80-
config = ConversionConfig(centroid=centroid, use_docker=use_docker)
81+
config = ConversionConfig(
82+
centroid=centroid,
83+
use_docker=use_docker,
84+
docker_image=docker_image,
85+
)
8186
mzml_path = run_msconvert(msconvert_path, job_raw, config)
8287

8388
# 3) Post-process

waters2mzml/pipeline.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ def run_pipeline(
2020
centroid: bool,
2121
skip_cleanup: bool = False,
2222
use_docker: bool = False,
23+
docker_image: str | None = None,
2324
do_postprocess: bool = True,
2425
) -> None:
2526
paths = default_paths(base_dir)
@@ -50,6 +51,7 @@ def run_pipeline(
5051
output_dir=paths.mzml_dir,
5152
centroid=centroid,
5253
use_docker=use_docker,
54+
docker_image=docker_image,
5355
do_postprocess=do_postprocess,
5456
)
5557

@@ -81,15 +83,12 @@ def run_pipeline_parallel(
8183
jobs: int,
8284
skip_cleanup: bool = False,
8385
use_docker: bool = False,
86+
docker_image: str | None = None,
8487
do_postprocess: bool = True,
8588
retries: int = 0,
8689
) -> None:
8790
"""
8891
Parallel version of run_pipeline using run_parallel.
89-
90-
- Same behavior as run_pipeline, but processes all RAW folders concurrently.
91-
- Uses the redesigned run_parallel with per-job isolation and retry logic.
92-
- Output is deterministic and matches the sequential pipeline's reporting style.
9392
"""
9493
paths = default_paths(base_dir)
9594
if input_dir is not None:
@@ -116,6 +115,7 @@ def run_pipeline_parallel(
116115
centroid=centroid,
117116
jobs=jobs,
118117
use_docker=use_docker,
118+
docker_image=docker_image,
119119
retries=retries,
120120
do_postprocess=do_postprocess,
121121
)

0 commit comments

Comments
 (0)