From cd487f6f0eb56e7a337d8fd569074558f303604c Mon Sep 17 00:00:00 2001 From: Ali Ramlaoui Date: Sat, 9 May 2026 14:55:13 +0200 Subject: [PATCH 1/5] docs: add README coverage guidance --- README.md | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/README.md b/README.md index e7af74e..4605a30 100644 --- a/README.md +++ b/README.md @@ -136,6 +136,33 @@ uv run --extra dev --locked --with "maturin>=1.4,<2.0" maturin develop uv run --extra dev --locked pytest ``` +### Coverage + +For Python coverage, install the extension module once, then run `pytest` with `pytest-cov` from the +existing `uv` environment: + +```bash +cd atompack-py +uv run --extra dev --locked --with "maturin>=1.4,<2.0" maturin develop +uv run --extra dev --locked --with pytest-cov \ + pytest tests --ignore=tests/benchmarks \ + --cov=atompack --cov-report=term-missing --cov-report=html +``` + +That measures the Python package surface, including calls that cross into the Rust extension through +the installed bindings. + +For Rust coverage, use `cargo-llvm-cov` at the workspace root: + +```bash +cargo install cargo-llvm-cov +cargo llvm-cov --workspace --html +``` + +This reports coverage for the Rust crates directly. In practice, the most useful workflow here is to +track both reports: Python coverage for the public API and integration paths, Rust coverage for the +storage and serialization core. + Rust entrypoints: ```bash From 773ab1d718417eeeddfa4a4f8d6912494d948dbd Mon Sep 17 00:00:00 2001 From: Ali Ramlaoui Date: Sat, 9 May 2026 15:00:10 +0200 Subject: [PATCH 2/5] ci: automate coverage reporting --- .github/scripts/coverage_summary.py | 82 ++++++++++++++++++++++++ .github/workflows/ci.yml | 99 +++++++++++++++++++++++++++++ Makefile | 20 +++++- README.md | 32 ++++------ 4 files changed, 212 insertions(+), 21 deletions(-) create mode 100644 .github/scripts/coverage_summary.py diff --git a/.github/scripts/coverage_summary.py b/.github/scripts/coverage_summary.py new file mode 100644 index 0000000..01a93cc --- /dev/null +++ b/.github/scripts/coverage_summary.py @@ -0,0 +1,82 @@ +#!/usr/bin/env python3 + +from __future__ import annotations + +import argparse +from dataclasses import dataclass +from pathlib import Path +import xml.etree.ElementTree as ET + + +@dataclass(frozen=True) +class CoverageStats: + name: str + covered: int + total: int + + @property + def percent(self) -> float: + if self.total == 0: + return 0.0 + return (self.covered / self.total) * 100.0 + + +def parse_python_coverage(path: Path) -> CoverageStats: + root = ET.parse(path).getroot() + total = int(root.attrib["lines-valid"]) + covered = int(root.attrib["lines-covered"]) + return CoverageStats(name="Python", covered=covered, total=total) + + +def parse_rust_coverage(path: Path) -> CoverageStats: + covered = 0 + total = 0 + + for line in path.read_text().splitlines(): + if line.startswith("LH:"): + covered += int(line[3:]) + elif line.startswith("LF:"): + total += int(line[3:]) + + return CoverageStats(name="Rust", covered=covered, total=total) + + +def build_summary(python_stats: CoverageStats, rust_stats: CoverageStats) -> str: + rows = "\n".join( + f"| {stats.name} | {stats.covered} | {stats.total} | {stats.percent:.2f}% |" + for stats in (python_stats, rust_stats) + ) + return "\n".join( + [ + "## Coverage Report", + "", + "| Scope | Covered Lines | Total Lines | Line Coverage |", + "| --- | ---: | ---: | ---: |", + rows, + "", + "Artifacts:", + "- Python XML/HTML coverage: `atompack-py/coverage/`", + "- Rust LCOV coverage: `coverage/rust.lcov`", + "", + ] + ) + + +def main() -> None: + parser = argparse.ArgumentParser(description="Build a markdown coverage summary.") + parser.add_argument("--python-xml", type=Path, required=True) + parser.add_argument("--rust-lcov", type=Path, required=True) + parser.add_argument("--output", type=Path, required=True) + args = parser.parse_args() + + python_stats = parse_python_coverage(args.python_xml) + rust_stats = parse_rust_coverage(args.rust_lcov) + summary = build_summary(python_stats, rust_stats) + + args.output.parent.mkdir(parents=True, exist_ok=True) + args.output.write_text(summary) + print(summary) + + +if __name__ == "__main__": + main() diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4a9b47d..11afc49 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -4,6 +4,9 @@ on: push: pull_request: +permissions: + contents: read + concurrency: group: ${{ github.workflow }}-${{ github.ref }} cancel-in-progress: true @@ -54,3 +57,99 @@ jobs: - name: Run checks run: make ci-py + + coverage: + name: Coverage + runs-on: ubuntu-latest + if: github.event_name == 'pull_request' + needs: + - rust + - python + permissions: + contents: read + issues: write + pull-requests: write + steps: + - uses: actions/checkout@v4 + + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: "3.11" + + - name: Set up uv + uses: astral-sh/setup-uv@v7 + with: + version: "0.9.16" + enable-cache: true + + - name: Set up Rust + uses: actions-rust-lang/setup-rust-toolchain@v1 + with: + toolchain: stable + components: llvm-tools-preview + + - name: Cargo cache + uses: Swatinem/rust-cache@v2 + + - name: Install cargo-llvm-cov + run: cargo install cargo-llvm-cov --locked + + - name: Run Python coverage + run: make py-coverage + + - name: Run Rust coverage + run: make rust-coverage + + - name: Build coverage summary + run: | + uv run --project atompack-py --no-sync python .github/scripts/coverage_summary.py \ + --python-xml atompack-py/coverage/python-coverage.xml \ + --rust-lcov coverage/rust.lcov \ + --output coverage/summary.md + cat coverage/summary.md >> "$GITHUB_STEP_SUMMARY" + + - name: Upload coverage artifacts + uses: actions/upload-artifact@v4 + with: + name: coverage-reports + path: | + atompack-py/coverage + coverage + + - name: Update pull request coverage comment + if: github.event.pull_request.head.repo.full_name == github.repository + uses: actions/github-script@v7 + with: + script: | + const fs = require("fs"); + const marker = ""; + const body = `${marker}\n${fs.readFileSync("coverage/summary.md", "utf8")}`; + const { owner, repo } = context.repo; + const issue_number = context.issue.number; + + const comments = await github.paginate(github.rest.issues.listComments, { + owner, + repo, + issue_number, + }); + + const existing = comments.find((comment) => + comment.user.type === "Bot" && comment.body.includes(marker) + ); + + if (existing) { + await github.rest.issues.updateComment({ + owner, + repo, + comment_id: existing.id, + body, + }); + } else { + await github.rest.issues.createComment({ + owner, + repo, + issue_number, + body, + }); + } diff --git a/Makefile b/Makefile index e7d7fd6..95f6a52 100644 --- a/Makefile +++ b/Makefile @@ -7,11 +7,11 @@ ATOMPACK_PERF_COLOR ?= always override UV_CACHE_DIR := $(CURDIR)/.uv-cache .PHONY: help \ - rust-fmt rust-fmt-check rust-lint rust-test \ - py-sync py-fmt py-fmt-check py-lint py-lint-fix py-dev py-dev-release py-test py-test-benchmarks \ + rust-fmt rust-fmt-check rust-lint rust-test rust-coverage \ + py-sync py-fmt py-fmt-check py-lint py-lint-fix py-dev py-dev-release py-test py-test-benchmarks py-coverage \ perf-smoke-rust perf-smoke-py perf-smoke \ docs-sync docs-build docs \ - fmt fmt-check lint test \ + fmt fmt-check lint test coverage \ ci-rust ci-py ci help: @@ -33,6 +33,9 @@ help: @echo " make py-dev uv maturin develop (atompack-py)" @echo " make py-dev-release uv maturin develop -r (atompack-py)" @echo " make perf-smoke Run opt-in Rust + Python release throughput smoke tests" + @echo " make py-coverage uv pytest-cov core suite with XML + HTML reports" + @echo " make rust-coverage cargo llvm-cov workspace report in coverage/rust.lcov" + @echo " make coverage Run both Python and Rust coverage targets" @echo "" @echo "Docs:" @echo " make docs-sync Install docs deps (uv, atompack-py docs group)" @@ -51,6 +54,11 @@ rust-lint: rust-test: cargo test --workspace +rust-coverage: + @command -v cargo-llvm-cov >/dev/null 2>&1 || (echo "cargo-llvm-cov not found; install with 'cargo install cargo-llvm-cov'" && exit 1) + mkdir -p coverage + cargo llvm-cov --workspace --lcov --output-path coverage/rust.lcov + py-sync: @command -v $(UV) >/dev/null 2>&1 || (echo "uv not found; install from https://docs.astral.sh/uv/" && exit 1) cd atompack-py && UV_CACHE_DIR=$(UV_CACHE_DIR) $(UV) sync --extra dev --locked @@ -96,6 +104,10 @@ perf-smoke-py: py-dev-release perf-smoke: perf-smoke-rust perf-smoke-py +py-coverage: py-dev + @command -v $(UV) >/dev/null 2>&1 || (echo "uv not found; install from https://docs.astral.sh/uv/" && exit 1) + cd atompack-py && mkdir -p coverage && UV_CACHE_DIR=$(UV_CACHE_DIR) $(UV) run --extra dev --locked --with pytest-cov pytest tests --ignore=tests/benchmarks --cov=atompack --cov-report=term-missing --cov-report=xml:coverage/python-coverage.xml --cov-report=html:coverage/htmlcov + docs-sync: @command -v $(UV) >/dev/null 2>&1 || (echo "uv not found; install from https://docs.astral.sh/uv/" && exit 1) UV_CACHE_DIR=$(UV_CACHE_DIR) $(UV) sync --project atompack-py --group docs --locked @@ -121,6 +133,8 @@ lint: rust-lint py-lint test: rust-test py-test +coverage: rust-coverage py-coverage + ci-rust: rust-fmt-check rust-lint rust-test ci-py: py-fmt-check py-lint py-test diff --git a/README.md b/README.md index 4605a30..83a62b1 100644 --- a/README.md +++ b/README.md @@ -138,30 +138,26 @@ uv run --extra dev --locked pytest ### Coverage -For Python coverage, install the extension module once, then run `pytest` with `pytest-cov` from the -existing `uv` environment: +Pull requests run an automated coverage job in GitHub Actions. That job: -```bash -cd atompack-py -uv run --extra dev --locked --with "maturin>=1.4,<2.0" maturin develop -uv run --extra dev --locked --with pytest-cov \ - pytest tests --ignore=tests/benchmarks \ - --cov=atompack --cov-report=term-missing --cov-report=html -``` - -That measures the Python package surface, including calls that cross into the Rust extension through -the installed bindings. +- runs Python coverage with `pytest-cov` +- runs Rust coverage with `cargo-llvm-cov` +- uploads the raw reports as workflow artifacts +- refreshes a sticky PR comment with the current Python and Rust line coverage -For Rust coverage, use `cargo-llvm-cov` at the workspace root: +For local runs, use the repo targets: ```bash -cargo install cargo-llvm-cov -cargo llvm-cov --workspace --html +make py-coverage +make rust-coverage +make coverage ``` -This reports coverage for the Rust crates directly. In practice, the most useful workflow here is to -track both reports: Python coverage for the public API and integration paths, Rust coverage for the -storage and serialization core. +`make py-coverage` writes XML and HTML reports under `atompack-py/coverage/`. `make rust-coverage` +writes an LCOV report to `coverage/rust.lcov`. + +In practice, the useful split here is to track both reports: Python coverage for the public API and +integration paths, Rust coverage for the storage and serialization core. Rust entrypoints: From b01508c146a20733e15160e659ce9140b6b0b94f Mon Sep 17 00:00:00 2001 From: Ali Ramlaoui Date: Sat, 9 May 2026 15:12:35 +0200 Subject: [PATCH 3/5] ci: switch coverage reporting to codecov badge --- .github/scripts/coverage_summary.py | 82 ----------------------------- .github/workflows/ci.yml | 65 +++++++---------------- README.md | 29 +++------- 3 files changed, 24 insertions(+), 152 deletions(-) delete mode 100644 .github/scripts/coverage_summary.py diff --git a/.github/scripts/coverage_summary.py b/.github/scripts/coverage_summary.py deleted file mode 100644 index 01a93cc..0000000 --- a/.github/scripts/coverage_summary.py +++ /dev/null @@ -1,82 +0,0 @@ -#!/usr/bin/env python3 - -from __future__ import annotations - -import argparse -from dataclasses import dataclass -from pathlib import Path -import xml.etree.ElementTree as ET - - -@dataclass(frozen=True) -class CoverageStats: - name: str - covered: int - total: int - - @property - def percent(self) -> float: - if self.total == 0: - return 0.0 - return (self.covered / self.total) * 100.0 - - -def parse_python_coverage(path: Path) -> CoverageStats: - root = ET.parse(path).getroot() - total = int(root.attrib["lines-valid"]) - covered = int(root.attrib["lines-covered"]) - return CoverageStats(name="Python", covered=covered, total=total) - - -def parse_rust_coverage(path: Path) -> CoverageStats: - covered = 0 - total = 0 - - for line in path.read_text().splitlines(): - if line.startswith("LH:"): - covered += int(line[3:]) - elif line.startswith("LF:"): - total += int(line[3:]) - - return CoverageStats(name="Rust", covered=covered, total=total) - - -def build_summary(python_stats: CoverageStats, rust_stats: CoverageStats) -> str: - rows = "\n".join( - f"| {stats.name} | {stats.covered} | {stats.total} | {stats.percent:.2f}% |" - for stats in (python_stats, rust_stats) - ) - return "\n".join( - [ - "## Coverage Report", - "", - "| Scope | Covered Lines | Total Lines | Line Coverage |", - "| --- | ---: | ---: | ---: |", - rows, - "", - "Artifacts:", - "- Python XML/HTML coverage: `atompack-py/coverage/`", - "- Rust LCOV coverage: `coverage/rust.lcov`", - "", - ] - ) - - -def main() -> None: - parser = argparse.ArgumentParser(description="Build a markdown coverage summary.") - parser.add_argument("--python-xml", type=Path, required=True) - parser.add_argument("--rust-lcov", type=Path, required=True) - parser.add_argument("--output", type=Path, required=True) - args = parser.parse_args() - - python_stats = parse_python_coverage(args.python_xml) - rust_stats = parse_rust_coverage(args.rust_lcov) - summary = build_summary(python_stats, rust_stats) - - args.output.parent.mkdir(parents=True, exist_ok=True) - args.output.write_text(summary) - print(summary) - - -if __name__ == "__main__": - main() diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 11afc49..001a6c4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -61,14 +61,13 @@ jobs: coverage: name: Coverage runs-on: ubuntu-latest - if: github.event_name == 'pull_request' + if: github.event_name == 'pull_request' || github.ref == 'refs/heads/main' needs: - rust - python permissions: contents: read - issues: write - pull-requests: write + id-token: write steps: - uses: actions/checkout@v4 @@ -101,13 +100,22 @@ jobs: - name: Run Rust coverage run: make rust-coverage - - name: Build coverage summary - run: | - uv run --project atompack-py --no-sync python .github/scripts/coverage_summary.py \ - --python-xml atompack-py/coverage/python-coverage.xml \ - --rust-lcov coverage/rust.lcov \ - --output coverage/summary.md - cat coverage/summary.md >> "$GITHUB_STEP_SUMMARY" + - name: Upload to Codecov (pull requests) + if: github.event_name == 'pull_request' + uses: codecov/codecov-action@v5 + with: + files: atompack-py/coverage/python-coverage.xml,coverage/rust.lcov + fail_ci_if_error: true + verbose: true + + - name: Upload to Codecov (pushes) + if: github.event_name == 'push' + uses: codecov/codecov-action@v5 + with: + files: atompack-py/coverage/python-coverage.xml,coverage/rust.lcov + fail_ci_if_error: true + use_oidc: true + verbose: true - name: Upload coverage artifacts uses: actions/upload-artifact@v4 @@ -116,40 +124,3 @@ jobs: path: | atompack-py/coverage coverage - - - name: Update pull request coverage comment - if: github.event.pull_request.head.repo.full_name == github.repository - uses: actions/github-script@v7 - with: - script: | - const fs = require("fs"); - const marker = ""; - const body = `${marker}\n${fs.readFileSync("coverage/summary.md", "utf8")}`; - const { owner, repo } = context.repo; - const issue_number = context.issue.number; - - const comments = await github.paginate(github.rest.issues.listComments, { - owner, - repo, - issue_number, - }); - - const existing = comments.find((comment) => - comment.user.type === "Bot" && comment.body.includes(marker) - ); - - if (existing) { - await github.rest.issues.updateComment({ - owner, - repo, - comment_id: existing.id, - body, - }); - } else { - await github.rest.issues.createComment({ - owner, - repo, - issue_number, - body, - }); - } diff --git a/README.md b/README.md index 83a62b1..fcb9b90 100644 --- a/README.md +++ b/README.md @@ -1,5 +1,11 @@ # Atompack +

+ + Codecov + +

+ Append-only molecule storage for atomistic ML datasets. Atompack is a Python package plus Rust core crate for writing, reading, and distributing molecular @@ -136,29 +142,6 @@ uv run --extra dev --locked --with "maturin>=1.4,<2.0" maturin develop uv run --extra dev --locked pytest ``` -### Coverage - -Pull requests run an automated coverage job in GitHub Actions. That job: - -- runs Python coverage with `pytest-cov` -- runs Rust coverage with `cargo-llvm-cov` -- uploads the raw reports as workflow artifacts -- refreshes a sticky PR comment with the current Python and Rust line coverage - -For local runs, use the repo targets: - -```bash -make py-coverage -make rust-coverage -make coverage -``` - -`make py-coverage` writes XML and HTML reports under `atompack-py/coverage/`. `make rust-coverage` -writes an LCOV report to `coverage/rust.lcov`. - -In practice, the useful split here is to track both reports: Python coverage for the public API and -integration paths, Rust coverage for the storage and serialization core. - Rust entrypoints: ```bash From 4b82df2cdbfbef36981d2b10a7e79b1023174939 Mon Sep 17 00:00:00 2001 From: Ali Ramlaoui Date: Sat, 9 May 2026 16:21:42 +0200 Subject: [PATCH 4/5] feat: support negative database indices --- atompack-py/python/atompack/__init__.pyi | 6 +-- atompack-py/python/atompack/_atompack_rs.pyi | 4 +- atompack-py/src/database.rs | 45 ++++++++++++++------ atompack-py/tests/test_database.py | 32 +++++++++++--- 4 files changed, 63 insertions(+), 24 deletions(-) diff --git a/atompack-py/python/atompack/__init__.pyi b/atompack-py/python/atompack/__init__.pyi index 67dddae..fd7b12f 100644 --- a/atompack-py/python/atompack/__init__.pyi +++ b/atompack-py/python/atompack/__init__.pyi @@ -534,7 +534,7 @@ class Database: Parameters ---------- index : int - Molecule index (0-based) + Molecule index (0-based). Negative indices are supported. Returns ------- @@ -549,7 +549,7 @@ class Database: Parameters ---------- indices : list of int - Molecule indices (0-based) + Molecule indices (0-based). Negative indices are supported. Returns ------- @@ -608,7 +608,7 @@ class Database: Parameters ---------- index : int - Molecule index (0-based) + Molecule index (0-based). Negative indices are supported. Returns ------- diff --git a/atompack-py/python/atompack/_atompack_rs.pyi b/atompack-py/python/atompack/_atompack_rs.pyi index 99e8730..3b4c284 100644 --- a/atompack-py/python/atompack/_atompack_rs.pyi +++ b/atompack-py/python/atompack/_atompack_rs.pyi @@ -475,7 +475,7 @@ class PyAtomDatabase: Parameters ---------- index : int - Molecule index (0-based) + Molecule index (0-based). Negative indices are supported. Returns ------- @@ -491,7 +491,7 @@ class PyAtomDatabase: Parameters ---------- indices : sequence of int - Molecule indices (0-based) + Molecule indices (0-based). Negative indices are supported. Returns ------- diff --git a/atompack-py/src/database.rs b/atompack-py/src/database.rs index f472cc9..5238d0a 100644 --- a/atompack-py/src/database.rs +++ b/atompack-py/src/database.rs @@ -11,6 +11,31 @@ pub(crate) struct PyAtomDatabase { } impl PyAtomDatabase { + fn normalize_index(&self, index: isize) -> PyResult { + let len = self.inner.len(); + let normalized = if index < 0 { + (len as isize) + .checked_add(index) + .ok_or_else(|| PyIndexError::new_err("index underflow"))? + } else { + index + }; + if normalized < 0 || normalized >= len as isize { + return Err(PyIndexError::new_err(format!( + "Index {} out of bounds for database of length {}", + index, len + ))); + } + Ok(normalized as usize) + } + + fn normalize_indices(&self, indices: Vec) -> PyResult> { + indices + .into_iter() + .map(|index| self.normalize_index(index)) + .collect() + } + fn single_molecule_view(&self, py: Python<'_>, index: usize) -> PyResult { let compression = self.inner.compression(); let use_mmap = self.inner.get_compressed_slice(0).is_some(); @@ -211,15 +236,9 @@ impl PyAtomDatabase { } /// Get a molecule by index as a lazy view-backed molecule. - fn get_molecule(&self, py: Python<'_>, index: usize) -> PyResult { - let len = self.inner.len(); - if index >= len { - return Err(PyIndexError::new_err(format!( - "Index {} out of bounds for database of length {}", - index, len - ))); - } - Ok(PyMolecule::from_view(self.single_molecule_view(py, index)?)) + fn get_molecule(&self, py: Python<'_>, index: isize) -> PyResult { + let normalized = self.normalize_index(index)?; + Ok(PyMolecule::from_view(self.single_molecule_view(py, normalized)?)) } /// Get multiple molecules by indices (parallel batch reading) @@ -232,10 +251,11 @@ impl PyAtomDatabase { /// /// Returns: /// - List of molecules - fn get_molecules(&self, py: Python<'_>, indices: Vec) -> PyResult> { + fn get_molecules(&self, py: Python<'_>, indices: Vec) -> PyResult> { if indices.is_empty() { return Ok(Vec::new()); } + let indices = self.normalize_indices(indices)?; let compression = self.inner.compression(); let use_mmap = self.inner.get_compressed_slice(0).is_some(); @@ -317,8 +337,9 @@ impl PyAtomDatabase { fn get_molecules_flat<'py>( &self, py: Python<'py>, - indices: Vec, + indices: Vec, ) -> PyResult> { + let indices = self.normalize_indices(indices)?; flat::get_molecules_flat_soa_impl(&self.inner, py, indices) } @@ -328,7 +349,7 @@ impl PyAtomDatabase { } /// Enable indexing: db[i] - fn __getitem__(&self, py: Python<'_>, index: usize) -> PyResult { + fn __getitem__(&self, py: Python<'_>, index: isize) -> PyResult { self.get_molecule(py, index) } diff --git a/atompack-py/tests/test_database.py b/atompack-py/tests/test_database.py index 6973049..de42ef1 100644 --- a/atompack-py/tests/test_database.py +++ b/atompack-py/tests/test_database.py @@ -248,6 +248,28 @@ def test_database_add_arrays_batch_roundtrip_with_custom_properties(tmp_path: Pa assert second.get_property("phase") == "valid" +def test_database_negative_indices_work_across_read_apis(tmp_path: Path) -> None: + path = tmp_path / "negative_indices.atp" + db = atompack.Database(str(path)) + db.add_molecules([_make_molecule(-1.0), _make_molecule(-2.0), _make_molecule(-3.0)]) + db.flush() + + reopened = atompack.Database.open(str(path)) + + assert reopened[-1].energy == pytest.approx(-3.0) + assert reopened.get_molecule(-2).energy == pytest.approx(-2.0) + assert [m.energy for m in reopened.get_molecules([-1, 0, -3])] == pytest.approx( + [-3.0, -1.0, -1.0] + ) + np.testing.assert_allclose( + reopened.get_molecules_flat([-1, -2])["energy"], + np.array([-3.0, -2.0], dtype=np.float64), + ) + + with pytest.raises(IndexError, match="out of bounds"): + reopened.get_molecule(-4) + + @pytest.mark.parametrize("mmap", [False, True]) @pytest.mark.parametrize("compression", ["none", "lz4", "zstd"]) def test_database_single_item_reads_are_view_compatible( @@ -596,19 +618,15 @@ def test_database_open_mmap_populate(tmp_path: Path) -> None: assert db_r[0].energy == pytest.approx(-3.0) -def test_database_negative_indexing_raises_overflow_error(tmp_path: Path) -> None: - # Database does not support negative indexing today. PyO3 extracts the - # index argument as `usize`, so a negative integer raises OverflowError - # at the FFI boundary. If wraparound semantics are ever added, this - # test will fail loudly so the intent is explicit. +def test_database_negative_indexing_out_of_bounds_raises_index_error(tmp_path: Path) -> None: path = tmp_path / "negidx.atp" db = atompack.Database(str(path)) db.add_molecule(_make_molecule(-1.0)) db.flush() db_r = atompack.Database.open(str(path)) - with pytest.raises(OverflowError, match=r"negative"): - _ = db_r[-1] + with pytest.raises(IndexError, match=r"out of bounds"): + _ = db_r[-2] def test_database_empty_molecule_roundtrip(tmp_path: Path) -> None: From 5aeb9f0a3958979d6e5ac28b9d268465c1b015bc Mon Sep 17 00:00:00 2001 From: Ali Ramlaoui Date: Sat, 9 May 2026 20:19:26 +0200 Subject: [PATCH 5/5] fix: stabilize PR CI checks --- .github/workflows/ci.yml | 5 +++-- atompack-py/src/database.rs | 4 +++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 001a6c4..b95345e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -105,7 +105,8 @@ jobs: uses: codecov/codecov-action@v5 with: files: atompack-py/coverage/python-coverage.xml,coverage/rust.lcov - fail_ci_if_error: true + token: ${{ secrets.CODECOV_TOKEN }} + fail_ci_if_error: false verbose: true - name: Upload to Codecov (pushes) @@ -113,7 +114,7 @@ jobs: uses: codecov/codecov-action@v5 with: files: atompack-py/coverage/python-coverage.xml,coverage/rust.lcov - fail_ci_if_error: true + fail_ci_if_error: false use_oidc: true verbose: true diff --git a/atompack-py/src/database.rs b/atompack-py/src/database.rs index 5238d0a..0da931f 100644 --- a/atompack-py/src/database.rs +++ b/atompack-py/src/database.rs @@ -238,7 +238,9 @@ impl PyAtomDatabase { /// Get a molecule by index as a lazy view-backed molecule. fn get_molecule(&self, py: Python<'_>, index: isize) -> PyResult { let normalized = self.normalize_index(index)?; - Ok(PyMolecule::from_view(self.single_molecule_view(py, normalized)?)) + Ok(PyMolecule::from_view( + self.single_molecule_view(py, normalized)?, + )) } /// Get multiple molecules by indices (parallel batch reading)