Conversation
Updated Python versions to only include 3.12 in the workflow.
Updated Python package workflow to use Python 3.11 and 3.12, and modified dependency installation.
There was a problem hiding this comment.
Pull request overview
This PR updates the project’s Python packaging/CI setup (targeting Python 3.12) and adjusts tests, docs, and a few implementation details to satisfy updated tooling/linting and schema/contract validation needs.
Changes:
- Adds/updates GitHub Actions workflows for packaging, linting (pylint), and PyPI publishing.
- Updates packaging metadata (rename distribution to
omenai, add URLs/dev deps) and refreshes README/quick-start install instructions. - Cleans up various code paths and tests (signature alignment, lint-driven refactors, adds JSON schema fixtures).
Reviewed changes
Copilot reviewed 34 out of 34 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
src/omen/scenario/contract_loader.py |
Replaces spec-specific loader with a generic load() that currently points at test fixtures. |
pyproject.toml |
Renames distribution to omenai, adjusts metadata, adds dev deps and project URLs. |
.github/workflows/python-package.yml |
Adds CI workflow for ruff, unit tests, and building artifacts on Python 3.12. |
.github/workflows/pylint.yml |
Adds pylint workflow on push. |
.github/workflows/release.yml |
Adds PyPI publish workflow on releases. |
.pylintrc |
Introduces repo-level pylint configuration. |
README.md, README.zh.md |
Updates project tagline/badges and installation instructions. |
docs/quick-start.md, docs/quick-start-llm.md |
Updates pip install commands and dev dependency guidance. |
tests/** |
Updates assertions/style, aligns monkeypatch signatures, adds contract fixture usage. |
tests/fixtures/contracts/*.schema.json |
Adds JSON schema fixtures used by tests/validators. |
src/omen/** (various) |
Minor refactors (string splitting, loops, unused vars/imports) and lint suppressions. |
app/case_analysis.py |
Renames a module-level variable used in the Streamlit UI flow. |
Comments suppressed due to low confidence (1)
src/omen/scenario/contract_loader.py:22
load()currently resolves schemas undertests/fixtures/contracts, and raises if the file is missing. Sincetests/typically isn’t included in sdist/wheels, this makes the loader unusable for library consumers and also contradicts the docstring (“repo spec path, with a test-fixture fallback”). Consider loading from the realspecs/.../contractslocation first, and only falling back totests/fixtures/contractswhen running in a dev/test context; also update the FileNotFoundError message (it still says “Spec 4”).
def load(schema_filename: str) -> dict:
"""Load a schema from the repo spec path, with a test-fixture fallback."""
root = Path(__file__).resolve().parents[3]
schema_path = (
root
/ "tests"
/ "fixtures"
/ "contracts"
/ schema_filename
)
if not schema_path.exists():
raise FileNotFoundError(f"Spec 4 contract schema not found: {schema_path}")
with schema_path.open("r", encoding="utf-8") as f:
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| / "fixtures" | ||
| / "contracts" |
There was a problem hiding this comment.
The schema_path path construction has inconsistent indentation around the / "fixtures" and / "contracts" segments, which is likely to trip style checks (ruff/pycodestyle) and makes the expression harder to read. Reformat the Path join to a consistently-indented style (or a single line) so it passes linting reliably.
| / "fixtures" | |
| / "contracts" | |
| / "fixtures" | |
| / "contracts" |
| def test_load_spec4_contract_schema() -> None: | ||
| schema = load_spec4_contract_schema("precision-evaluation.schema.json") | ||
| schema = load("precision-evaluation.schema.json") | ||
| assert schema["title"] == "PrecisionEvaluationProfile" |
There was a problem hiding this comment.
The test name test_load_spec4_contract_schema no longer matches what’s being exercised (it now calls contract_loader.load). Renaming the test to reflect the new API will keep intent clear and avoid confusion when grepping failures.
| @@ -109,11 +113,17 @@ More scenarios are under development (contributions welcome): | |||
|
|
|||
| Environment requirements: Python 3.12+ with `pip` package manager. | |||
There was a problem hiding this comment.
README says “Python 3.12+”, but pyproject.toml pins requires-python = ">=3.12,<3.13", which excludes 3.13+. Please align the wording (e.g., “Python 3.12.x”) so users don’t try unsupported versions.
| Environment requirements: Python 3.12+ with `pip` package manager. | |
| Environment requirements: Python 3.12.x with `pip` package manager. |
| @@ -109,11 +113,17 @@ graph TD | |||
|
|
|||
| 运行环境要求:Python 3.12+ `pip` 包管理器。 | |||
There was a problem hiding this comment.
文档写的是 “Python 3.12+”,但 pyproject.toml 里 requires-python = ">=3.12,<3.13" 实际不支持 3.13+。建议把这里的表述改成 “Python 3.12.x” 或与实际支持范围一致,避免用户装错版本。
| 运行环境要求:Python 3.12+ `pip` 包管理器。 | |
| 运行环境要求:Python 3.12.x 和 `pip` 包管理器。 |
| requires-python = ">=3.12,<3.13" | ||
| authors = [{ name = "StrategyLogic" }] | ||
| license = { text = "AGPL-3.0-or-later" } | ||
| license = "AGPL-3.0-or-later" |
There was a problem hiding this comment.
[project].license was changed from the PEP 621 table form ({ text = ... }) to a plain string. Many Python packaging tools expect license to be a table in pyproject.toml; using a string here can break python -m build / metadata generation depending on tool versions. To stay standards-compliant and avoid release workflow failures, use the table form (or license = { file = "LICENSE" }) instead of a bare string.
| license = "AGPL-3.0-or-later" | |
| license = { text = "AGPL-3.0-or-later" } |
No description provided.