From 6f6c5ce6718a7513c7ca78d2d562dab744b862ed Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Thu, 3 Sep 2026 13:37:01 -0400 Subject: [PATCH] Open dataset HDF5 files read-only when loading pandas opens HDFStore in append mode by default, so loading a dataset from a read-only file (a Hugging Face cache snapshot, a read-only mount, or a chmod 444 copy) failed with PermissionError before any data was read. Pass mode="r" at both read sites in UKSingleYearDataset and UKMultiYearDataset; the two save() sites keep write access. Add regression tests that load synthetic single- and multi-year datasets from read-only files without network access, plus a microsimulation test that loads a read-only copy of enhanced_frs_2023_24_tiny.h5 and computes household_net_income. Co-Authored-By: Claude Fable 5.1 --- changelog.d/hdfstore-read-only.fixed.md | 1 + policyengine_uk/data/dataset_schema.py | 4 +- .../tests/test_read_only_dataset.py | 125 ++++++++++++++++++ 3 files changed, 128 insertions(+), 2 deletions(-) create mode 100644 changelog.d/hdfstore-read-only.fixed.md create mode 100644 policyengine_uk/tests/test_read_only_dataset.py diff --git a/changelog.d/hdfstore-read-only.fixed.md b/changelog.d/hdfstore-read-only.fixed.md new file mode 100644 index 0000000000..570ff4bb5a --- /dev/null +++ b/changelog.d/hdfstore-read-only.fixed.md @@ -0,0 +1 @@ +- Open dataset HDF5 files in read-only mode when loading `UKSingleYearDataset` and `UKMultiYearDataset`, so datasets in read-only locations (such as Hugging Face cache snapshots or read-only mounts) load instead of raising a `PermissionError` from pandas' default append mode. diff --git a/policyengine_uk/data/dataset_schema.py b/policyengine_uk/data/dataset_schema.py index d2fa19a748..8e941d2708 100644 --- a/policyengine_uk/data/dataset_schema.py +++ b/policyengine_uk/data/dataset_schema.py @@ -51,7 +51,7 @@ def __init__( file_path = str(file_path) if file_path else None if file_path is not None: self.validate_file_path(file_path) - with pd.HDFStore(file_path) as f: + with pd.HDFStore(file_path, mode="r") as f: self._person = f["person"] self._benunit = f["benunit"] self._household = f["household"] @@ -193,7 +193,7 @@ def __init__( if file_path is not None: UKMultiYearDataset.validate_file_path(file_path) - with pd.HDFStore(file_path) as f: + with pd.HDFStore(file_path, mode="r") as f: self.datasets = {} for year in f.keys(): if year.startswith("/person/"): diff --git a/policyengine_uk/tests/test_read_only_dataset.py b/policyengine_uk/tests/test_read_only_dataset.py new file mode 100644 index 0000000000..1ff3e9d79a --- /dev/null +++ b/policyengine_uk/tests/test_read_only_dataset.py @@ -0,0 +1,125 @@ +"""Datasets must load from read-only files. + +pandas opens ``HDFStore`` in append mode by default, which raises a +``PermissionError`` when the file is not writable. Datasets are routinely +read from such locations: Hugging Face cache snapshots are checked out +read-only, and shared datasets may live on read-only mounts. Loading a dataset +never writes to it, so every read path must open the store with ``mode="r"``. + +The synthetic tests below need no network access. The microsimulation test +downloads the tiny enhanced FRS dataset, so it is skipped (via the +``microsimulation`` marker in ``conftest.py``) unless ``HUGGING_FACE_TOKEN`` +or ``POLICYENGINE_UK_DEFAULT_DATASET`` is set. +""" + +from __future__ import annotations + +import hashlib +import os +import shutil +import stat +from pathlib import Path + +import numpy as np +import pandas as pd +import pytest + +from policyengine_uk.data import UKMultiYearDataset, UKSingleYearDataset + +TINY_DATASET_REPO = "policyengine/policyengine-uk-data-private" +TINY_DATASET_FILENAME = "enhanced_frs_2023_24_tiny.h5" +TINY_DATASET_VERSION = "1.57.2" +YEAR = 2025 + + +def _make_read_only(path: Path) -> None: + """Strip write permission from *path*, or skip if that has no effect. + + Running as root (some containers) ignores file permission bits, which + would let the test pass without exercising the read-only code path. + """ + path.chmod(stat.S_IRUSR | stat.S_IRGRP | stat.S_IROTH) + if os.access(path, os.W_OK): + pytest.skip("Cannot make files read-only in this environment") + + +def _sha256(path: Path) -> str: + return hashlib.sha256(path.read_bytes()).hexdigest() + + +def _synthetic_dataset(fiscal_year: int) -> UKSingleYearDataset: + person = pd.DataFrame( + { + "person_id": [1, 2, 3], + "person_benunit_id": [1, 1, 2], + "person_household_id": [1, 1, 2], + "age": [40.0, 38.0, 70.0], + } + ) + benunit = pd.DataFrame({"benunit_id": [1, 2]}) + household = pd.DataFrame({"household_id": [1, 2]}) + return UKSingleYearDataset( + person=person, + benunit=benunit, + household=household, + fiscal_year=fiscal_year, + ) + + +def test_single_year_dataset_loads_from_read_only_file(tmp_path: Path): + path = tmp_path / "single_year.h5" + _synthetic_dataset(YEAR).save(str(path)) + _make_read_only(path) + digest = _sha256(path) + + loaded = UKSingleYearDataset(str(path)) + + assert loaded.time_period == str(YEAR) + assert list(loaded.person["person_id"]) == [1, 2, 3] + assert list(loaded.household["household_id"]) == [1, 2] + assert _sha256(path) == digest, "Loading must not modify the dataset file" + + +def test_multi_year_dataset_loads_from_read_only_file(tmp_path: Path): + path = tmp_path / "multi_year.h5" + UKMultiYearDataset( + datasets=[_synthetic_dataset(YEAR), _synthetic_dataset(YEAR + 1)] + ).save(str(path)) + _make_read_only(path) + digest = _sha256(path) + + loaded = UKMultiYearDataset(str(path)) + + assert loaded.years == [YEAR, YEAR + 1] + assert list(loaded[YEAR + 1].person["age"]) == [40.0, 38.0, 70.0] + assert _sha256(path) == digest, "Loading must not modify the dataset file" + + +@pytest.mark.microsimulation +def test_microsimulation_loads_read_only_tiny_dataset(tmp_path: Path): + """Reproduces loading from a Hugging Face cache snapshot. + + Snapshot files are read-only on disk; before the fix, + ``Microsimulation(dataset=path)`` raised a ``PermissionError`` on open. + """ + from huggingface_hub import hf_hub_download + + from policyengine_uk import Microsimulation + + source = hf_hub_download( + repo_id=TINY_DATASET_REPO, + filename=TINY_DATASET_FILENAME, + revision=TINY_DATASET_VERSION, + token=os.environ.get("HUGGING_FACE_TOKEN") or None, + ) + path = tmp_path / TINY_DATASET_FILENAME + shutil.copyfile(source, path) + _make_read_only(path) + digest = _sha256(path) + + sim = Microsimulation(dataset=str(path)) + net_income = np.asarray(sim.calculate("household_net_income", YEAR).values) + + assert len(net_income) > 0 + assert np.isfinite(net_income).all() + assert _sha256(path) == digest, "Loading must not modify the dataset file"