Add measles ABM template support (supersedes #16) - #24
Open
jonathanhhb wants to merge 6 commits into
Open
Conversation
Resolves three conflicts against main since the PR opened: - tests/test_models.py: keep measles/measles_plot imports; adopt main's isort-style alphabetical ordering. - CHANGELOG.md: keep measles-ABM Added entries alongside main's docs overhaul and registry entries. - src/laser/init/cli.py: main refactored the loader dispatch dict into registry.get_model_loader(mode); drop the hardcoded "ABM/MEASLES" entry (MEASLES now flows through the mode-keyed registry and the click.Choice validation) and update the comment to list MEASLES.
added 2 commits
July 31, 2026 09:47
Two CI failures on PR #24: - Lint (Python 3.12): E741 on measles_plot.py:60 (I = np.array(...)). Kept the SEIR-standard variable name with an inline noqa noting the domain convention. - Tests (3.10 + 3.14): coverage 89.36% < 90% threshold, because measles.py and measles_plot.py are template files (copied verbatim to user output dirs, not exercised by laser-init's own tests). Existing templates (si/sir/seir/plot.py) are already excluded from coverage; extended the same omit list to the two measles templates.
There was a problem hiding this comment.
🟡 Not ready to approve
It introduces a couple of correctness/convention issues in the new measles plotting/template code (notably an incorrect return type and Click path handling inconsistencies) that should be fixed before merging.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
Adds first-class support for a measles-specific ABM template flow, allowing laser-init to emit measles model scripts/config via the existing ABM loader path and exposing MEASLES as a CLI --model choice.
Changes:
- Added measles ABM template + plotting helper modules (
measles.py,measles_plot.py) built onlaser-measles. - Extended ABM loader to emit measles-specific scripts/config, and extended CLI model choices to include
MEASLES. - Added runtime deps (
laser-measles,polars) plus tests validating module presence and ABM emission behavior.
File summaries
| File | Description |
|---|---|
| tests/test_models.py | Verifies measles template modules can be imported from laser.init.models. |
| tests/test_loaders.py | Adds ABM loader tests to ensure MEASLES emits the expected files and config keys. |
| src/laser/init/models/measles.py | New measles ABM template script that runs a laser-measles simulation and invokes plotting. |
| src/laser/init/models/measles_plot.py | New measles plotting helper that generates a single PDF report from multiple figures. |
| src/laser/init/models/init.py | Updates package docstring to mention measles templates. |
| src/laser/init/loaders/abm.py | Adds measles-specific config template and emission path (_emit_measles) with logging. |
| src/laser/init/cli.py | Adds MEASLES to the CLI --model choice and updates docstrings/comments accordingly. |
| pyproject.toml | Declares new runtime dependencies and excludes measles template modules from coverage. |
| CHANGELOG.md | Documents the new measles ABM template support and new dependencies. |
Review details
Suppressed comments (1)
src/laser/init/models/measles.py:32
- Other model templates use click.Path(..., path_type=Path) so Click passes Path objects into main(); measles.py currently omits path_type for --data-dir, diverging from the established pattern (e.g., src/laser/init/models/seir.py:33).
type=click.Path(exists=True),
- Files reviewed: 9/9 changed files
- Comments generated: 4
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Comment on lines
+9
to
+11
| def show_plots( | ||
| model, scenario: pl.DataFrame, output_dir: Path | None, name: str = "measles" | ||
| ) -> Path: |
Comment on lines
+40
to
+42
| else: | ||
| pdf_path = None | ||
|
|
Comment on lines
+78
to
+81
| Creates a YAML configuration file with data file paths and simulation parameters, | ||
| then copies the appropriate model script (SI, SIR, or SEIR) and plotting utilities | ||
| to the output directory. | ||
| then copies the appropriate model script (SI, SIR, SEIR, or MEASLES) and plotting | ||
| utilities to the output directory. The MEASLES model uses laser-measles instead of | ||
| laser-generic and requires only shape and crude rate data files. |
- measles.py: add path_type=Path to both click.Path options so main() receives Path objects, matching the pattern in seir.py/sir.py/si.py. - measles_plot.py: correct return type to Path | None; close figures in the no-output-dir path (moved plt.close outside the if/else so every figure is closed regardless of whether it was written to PDF). - loaders/abm.py: docstring now notes that pop_filename/exp_filename are accepted for signature compatibility but ignored for MEASLES.
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.
Supersedes #16 by @krosenfeld-IDM — same content, rebased on current
mainwith three conflicts resolved.Summary
models/measles.py) and plotting helpers (models/measles_plot.py) built onlaser-measlesMEASLESas a--modelchoice in the CLI, routed through the existingABMloader modelaser-measles>=0.10.0,polars>=1.0.0AbmLoader.emit_script(model="MEASLES")file emission and config keysConflict resolutions vs #16
Three conflicts against main since April:
tests/test_models.py— adopt main's isort-style alphabetical import order (keepmeasles,measles_plot).CHANGELOG.md— concatenate measles-ABM Added entries with main's docs-overhaul/registry entries under the same### Addedheading.src/laser/init/cli.py— main refactored the loader dispatch dict intoregistry.get_model_loader(mode)(); the PR's"ABM/MEASLES": abm.AbmLoaderdict entry is no longer needed (MEASLES flows through the mode-keyed registry, with model-type validation fromclick.Choice).Test plan
Credit for the underlying work: @krosenfeld-IDM.