diff --git a/book/marimo/notebooks/optimize.py b/book/marimo/notebooks/optimize.py index c848bc6f..0181232d 100644 --- a/book/marimo/notebooks/optimize.py +++ b/book/marimo/notebooks/optimize.py @@ -12,6 +12,7 @@ from __future__ import annotations import argparse +import sys import warnings from collections.abc import Callable from functools import cache @@ -292,8 +293,15 @@ def optimize(key: str, *, n_trials: int, seed: int) -> optuna.Study: return study -def main(argv: list[str] | None = None) -> None: - """Parse command-line arguments and run the requested Optuna study/studies.""" +def main(argv: list[str] | None = None) -> int: + """Parse command-line arguments and run the requested Optuna study/studies. + + Returns the process exit code: ``0`` on success, ``1`` when the price data the + experiments read is missing. That case is caught here rather than allowed to + propagate because it is the one failure a user hits through no fault of the + search — a setup problem deserving a one-line message naming the file, not a + traceback out of the loader. + """ parser = argparse.ArgumentParser(description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter) parser.add_argument( "--experiment", @@ -315,10 +323,15 @@ def main(argv: list[str] | None = None) -> None: if not args.verbose: optuna.logging.set_verbosity(optuna.logging.WARNING) keys = list(EXPERIMENTS.keys()) if args.experiment == "all" else [args.experiment] - for key in keys: - n_trials = args.trials if args.trials is not None else DEFAULT_TRIALS[key] - optimize(key, n_trials=n_trials, seed=args.seed) + try: + for key in keys: + n_trials = args.trials if args.trials is not None else DEFAULT_TRIALS[key] + optimize(key, n_trials=n_trials, seed=args.seed) + except FileNotFoundError as error: + print(f"error: {error}", file=sys.stderr) + return 1 + return 0 if __name__ == "__main__": - main() + sys.exit(main()) diff --git a/book/marimo/notebooks/preamble.py b/book/marimo/notebooks/preamble.py index 3c3d6383..57dd9076 100644 --- a/book/marimo/notebooks/preamble.py +++ b/book/marimo/notebooks/preamble.py @@ -23,6 +23,9 @@ #: Directory holding the marimo notebooks (this file's own directory). NOTEBOOK_DIR = Path(__file__).resolve().parent +#: Price file every notebook reads, from the ``public/`` directory beside it. +PRICES_CSV = "Prices_hashed.csv" + def load_notebook(name: str) -> dict[str, Any]: """Execute sibling notebook ``name`` (e.g. ``"Experiment1.py"``) and return its namespace. @@ -56,8 +59,21 @@ def load_prices(notebook_file: str) -> pl.DataFrame: Datetime(time_unit='ns', time_zone=None) >>> set(prices.drop(date_col).dtypes) == {pl.Float64} True + + An absent CSV is checked for here rather than left to ``pl.read_csv``, so the + error names the file that was looked for — the file ships with the repository, + so its absence is a setup problem with an obvious remedy: + + >>> try: + ... load_prices(str(NOTEBOOK_DIR / "elsewhere" / "preamble.py")) + ... except FileNotFoundError as error: + ... PRICES_CSV in str(error) + True """ - path = Path(notebook_file).parent / "public" / "Prices_hashed.csv" + path = Path(notebook_file).parent / "public" / PRICES_CSV + if not path.is_file(): + msg = f"Price data not found: {path} — it ships with the repository; check the checkout is complete." + raise FileNotFoundError(msg) dframe = pl.read_csv(str(path), try_parse_dates=True) dframe = dframe.with_columns(pl.col(date_col).cast(pl.Datetime("ns"))) dframe = dframe.with_columns([pl.col(col).cast(pl.Float64) for col in dframe.columns if col != date_col]) diff --git a/tests/test_optimize.py b/tests/test_optimize.py index 88042f1e..112987bf 100644 --- a/tests/test_optimize.py +++ b/tests/test_optimize.py @@ -96,13 +96,37 @@ def _raise(*_args, **_kwargs): def test_main_runs_single_experiment(capsys): - """The CLI entry point runs one experiment and prints its summary.""" - optimize["main"](["--experiment", "1", "--trials", "1"]) + """The CLI entry point runs one experiment, prints its summary and reports success.""" + assert optimize["main"](["--experiment", "1", "--trials", "1"]) == 0 out = capsys.readouterr().out assert "Experiment 1" in out assert "best Sharpe" in out +def test_main_reports_missing_price_data_without_a_traceback(capsys, monkeypatch): + """Absent price data exits non-zero with a message naming the file, not a traceback. + + ``load_prices`` raises ``FileNotFoundError`` when the CSV is gone; ``main`` is the + only entry point a user drives directly, so it must turn that into a one-line + diagnostic. Patching the module-global ``optimize`` (the same ``__globals__`` + technique the singular-matrix and zero-baseline tests use) raises the error from + inside the loop without deleting the file the rest of the suite reads. + """ + + def _raise(*_args, **_kwargs): + """Stand in for the study runner, failing the way a missing price CSV does.""" + msg = "Price data not found: /nowhere/public/Prices_hashed.csv" + raise FileNotFoundError(msg) + + main = optimize["main"] + monkeypatch.setitem(main.__globals__, "optimize", _raise) + + assert main(["--experiment", "1", "--trials", "1"]) == 1 + captured = capsys.readouterr() + assert "Prices_hashed.csv" in captured.err + assert "Traceback" not in captured.err + + def test_default_trials_cover_every_experiment(): """DEFAULT_TRIALS has an entry for each registered experiment (no KeyError in main).""" assert set(optimize["DEFAULT_TRIALS"]) == set(optimize["EXPERIMENTS"]) diff --git a/tests/test_preamble.py b/tests/test_preamble.py index 77147534..665fdc95 100644 --- a/tests/test_preamble.py +++ b/tests/test_preamble.py @@ -12,6 +12,7 @@ preamble = load_notebook("preamble.py") load_prices = preamble["load_prices"] date_col = preamble["date_col"] +PRICES_CSV = preamble["PRICES_CSV"] @pytest.fixture(scope="module") @@ -58,6 +59,18 @@ def test_load_prices_non_date_columns_are_float64(prices_df): assert prices_df[col].dtype == pl.Float64, f"Column {col!r} is not Float64" +def test_load_prices_missing_csv_names_the_expected_path(tmp_path): + """A missing price CSV raises FileNotFoundError naming the file that was looked for. + + ``tmp_path`` stands in for a caller sitting outside the notebook directory, so + the ``public/`` sibling the loader resolves does not exist. The guard must fire + before ``pl.read_csv``, whose own error names neither the expected location nor + the fact that the file ships with the repository. + """ + with pytest.raises(FileNotFoundError, match=PRICES_CSV): + load_prices(str(tmp_path / "Experiment1.py")) + + def test_load_prices_interpolation_applied(prices_df): """load_prices applies interpolation to the raw data.""" from jquantstats import interpolate