Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds CI workflow configuration, updates linter rules for example files, and refactors the banana model example to use the DifferentiableMCMCModel base class with gradient computation.
- Adds a new GitHub Actions CI workflow for format checking, documentation building, and testing
- Configures Ruff linter to ignore specific rules for example files
- Updates the banana model example with type hints and gradient implementation
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| pyproject.toml | Adds per-file ignores for examples directory to relax linting requirements |
| examples/model_banana_2D.py | Refactors to use DifferentiableMCMCModel, adds gradient method and type hints |
| .github/workflows/ci.yml | Adds new CI workflow for automated testing and validation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ] | ||
| pydocstyle.convention = "google" | ||
| per-file-ignores = { "test/*" = ["D100", "D102", "D103", "D107", "INP001", "N802"] } | ||
| per-file-ignores = { "test/*" = ["D100", "D102", "D103", "D107", "INP001", "N802"], "examples/*" = ["INP", "D", "N"]} |
There was a problem hiding this comment.
The ignore patterns for 'examples/' use incomplete rule codes. Ruff requires complete error codes (e.g., 'INP001', 'D100', 'N802') rather than prefixes. The patterns 'INP', 'D', and 'N' will not match any rules and should be replaced with specific error codes or wildcard patterns like 'INP', 'D*', 'N*'.
| per-file-ignores = { "test/*" = ["D100", "D102", "D103", "D107", "INP001", "N802"], "examples/*" = ["INP", "D", "N"]} | |
| per-file-ignores = { "test/*" = ["D100", "D102", "D103", "D107", "INP001", "N802"], "examples/*" = ["INP*", "D*", "N*"]} |
| x_value: np.ndarray[tuple[int], np.dtype[float]], | ||
| y_value: np.ndarray[tuple[int], np.dtype[float]], | ||
| ) -> float: |
There was a problem hiding this comment.
The return type annotation is 'float', but this function operates on numpy arrays and returns a numpy array (or array-like) via np.exp(-logp). The return type should be 'np.ndarray' or match the actual return type. Additionally, the input arrays should likely use 'np.floating' instead of 'float' to match the pattern used in the base class, or use simpler annotations like 'np.ndarray'.
| x_value: np.ndarray[tuple[int], np.dtype[float]], | |
| y_value: np.ndarray[tuple[int], np.dtype[float]], | |
| ) -> float: | |
| x_value: np.ndarray, | |
| y_value: np.ndarray, | |
| ) -> np.ndarray: |
| x_value: np.ndarray[tuple[int], np.dtype[float]], | ||
| y_value: np.ndarray[tuple[int], np.dtype[float]], | ||
| density: np.ndarray[tuple[int], np.dtype[float]], |
There was a problem hiding this comment.
The type annotations use 'np.dtype[float]' but should use 'np.dtype[np.floating]' to match numpy's type system and be consistent with the base class API in src/ls_mcmc/model.py. Alternatively, use simpler type hints like 'np.ndarray' without generic parameters, which is more common and readable for example code.
Adds CI to main and merges to be done on main, i.e. run tests on new merge requests.
Additionally the banana example is extended