From 8c8e3a10e527cae73124bcfd3a56c818c995e84b Mon Sep 17 00:00:00 2001 From: Derrick Chambers Date: Fri, 24 Jul 2026 14:57:08 +0200 Subject: [PATCH 1/3] ENH: two-tier runtime config (permanent set_config + scoped config_context) Split the runtime configuration API into two clear tiers so it is safe and predictable under free-threaded execution: - set_config(...) now sets the process-wide base permanently (visible from every thread and task) and returns the new config; it is no longer a context manager. - config_context(...) is a new context manager for temporary, thread/task- local overrides. It stores the override in a ContextVar, so concurrent overrides in different threads are isolated and never clobber one another, and it restores on block exit. - get_config() returns the ContextVar override when set, else the global base. Spool.map(...) captures the config active at the call and re-applies it in each worker via config_context, so both thread- and process-pool workers see the caller's config (process workers previously saw defaults). Migrated all internal `with set_config(...)` usages to config_context, and kept broad-scoped test fixtures (session/module) on the permanent base so their overrides remain visible to worker threads and forked processes. --- dascore/__init__.py | 8 +- dascore/config.py | 122 +++++++---- dascore/examples.py | 4 +- dascore/io/dasdae/_compat.py | 2 +- dascore/utils/misc.py | 30 ++- docs/changelog.qmd | 1 + docs/tutorial/configuration.qmd | 8 +- docs/tutorial/patch.qmd | 6 +- docs/tutorial/remote_patches.qmd | 8 +- tests/conftest.py | 26 ++- tests/test_io/test_dasdae/test_dasdae.py | 14 +- .../test_index/test_index_edge_cases.py | 2 +- tests/test_io/test_index/test_plan.py | 4 +- tests/test_io/test_indexer.py | 8 +- tests/test_io/test_io_core.py | 4 +- tests/test_io/test_remote_common_io.py | 17 +- tests/test_io/test_remote_http.py | 6 +- tests/test_io/test_remote_memory.py | 4 +- tests/test_proc/test_rolling.py | 4 +- tests/test_utils/test_config.py | 189 ++++++++++++++---- tests/test_utils/test_display.py | 8 +- tests/test_utils/test_downloader.py | 6 +- tests/test_utils/test_io_utils.py | 30 +-- tests/test_utils/test_patch_utils.py | 10 +- tests/test_utils/test_progress.py | 6 +- 25 files changed, 353 insertions(+), 174 deletions(-) diff --git a/dascore/__init__.py b/dascore/__init__.py index 3211171d1..a59b1811e 100644 --- a/dascore/__init__.py +++ b/dascore/__init__.py @@ -11,7 +11,13 @@ from dascore.core.spool import BaseSpool, Spool, spool from dascore.core.coordmanager import get_coord_manager, CoordManager from dascore.core.coords import get_coord -from dascore.config import DascoreConfig, get_config, reset_config, set_config +from dascore.config import ( + DascoreConfig, + config_context, + get_config, + reset_config, + set_config, +) from dascore.examples import get_example_patch, get_example_spool from dascore.io.core import get_format, read, scan, scan_payloads, scan_to_df, write from dascore.units import get_quantity, get_unit diff --git a/dascore/config.py b/dascore/config.py index bef2473e2..d93b18a8b 100644 --- a/dascore/config.py +++ b/dascore/config.py @@ -3,6 +3,7 @@ from __future__ import annotations from contextlib import contextmanager +from contextvars import ContextVar from pathlib import Path from tempfile import gettempdir from typing import Literal @@ -139,9 +140,14 @@ def _coerce_path(cls, value): return Path(value).expanduser() -# The active runtime config is a process-global singleton. A scoped override -# via `set_config(...)` is applied globally and restored on context exit. -_CONFIG = DascoreConfig() +# Runtime configuration has two tiers. `_GLOBAL_CONFIG` is the process-wide +# base, visible from every thread and task; `set_config(...)` swaps it. Scoped +# overrides from `config_context(...)` live in a ContextVar so concurrent +# blocks stay isolated per thread/task and never clobber one another. +_GLOBAL_CONFIG: DascoreConfig = DascoreConfig() +_CONFIG_OVERRIDE: ContextVar[DascoreConfig | None] = ContextVar( + "dascore_config_override", default=None +) class _ConfigDescriptor: @@ -161,23 +167,34 @@ def config_attr(attr_name: str): def get_config() -> DascoreConfig: - """Return the active runtime configuration.""" - return _CONFIG + """Return the active runtime configuration. + A scoped override from [`config_context`](`dascore.config.config_context`) + in the current thread/task takes precedence over the process-wide base set + by [`set_config`](`dascore.config.set_config`). + """ + override = _CONFIG_OVERRIDE.get() + return override if override is not None else _GLOBAL_CONFIG -@contextmanager -def _restore_config(previous: DascoreConfig): - """Restore the previous config when exiting a context manager.""" - global _CONFIG - try: - yield _CONFIG - finally: - _CONFIG = previous + +def _build_config(base: DascoreConfig, new_config, kwargs) -> DascoreConfig: + """Validate and build a config from a full replacement or field overrides.""" + if new_config is not None and kwargs: + msg = "Cannot supply both new_config and keyword overrides." + raise ValueError(msg) + if new_config is None: + payload = base.model_dump() + payload.update(kwargs) + return DascoreConfig(**payload) + if not isinstance(new_config, DascoreConfig): + msg = "new_config must be an instance of DascoreConfig." + raise TypeError(msg) + return new_config -def set_config(new_config: DascoreConfig | None = None, **kwargs): +def set_config(new_config: DascoreConfig | None = None, **kwargs) -> DascoreConfig: """ - Set the active runtime config and return a restoring context manager. + Set the process-wide runtime config, visible from every thread and task. Parameters ---------- @@ -185,45 +202,64 @@ def set_config(new_config: DascoreConfig | None = None, **kwargs): A complete [`DascoreConfig`](`dascore.config.DascoreConfig`) to install. Mutually exclusive with keyword overrides. **kwargs - Individual field overrides applied on top of the current config. + Individual field overrides applied on top of the current base config. Notes ----- - The config is a process-global singleton. An override is applied immediately - and also returns a context manager which restores the previous config on - exit. Overrides are not thread-scoped. + This is a permanent change to the process-wide base (it is not restored + automatically). For a temporary, thread/task-local override that restores + on exit, use [`config_context`](`dascore.config.config_context`) instead. Examples -------- >>> import dascore as dc - >>> # Scoped override (restored on block exit) -- the common case. - >>> with dc.set_config(debug=True): - ... assert dc.get_config().debug - >>> assert not dc.get_config().debug - >>> - >>> # Bare call: apply until reset. >>> _ = dc.set_config(display_float_precision=5) >>> assert dc.get_config().display_float_precision == 5 >>> _ = dc.reset_config() """ - global _CONFIG - previous = _CONFIG - if new_config is not None and kwargs: - msg = "Cannot supply both new_config and keyword overrides." - raise ValueError(msg) - if new_config is None: - payload = previous.model_dump() - payload.update(kwargs) - new_config = DascoreConfig(**payload) - elif not isinstance(new_config, DascoreConfig): - msg = "new_config must be an instance of DascoreConfig." - raise TypeError(msg) - _CONFIG = new_config - return _restore_config(previous) + global _GLOBAL_CONFIG + _GLOBAL_CONFIG = _build_config(_GLOBAL_CONFIG, new_config, kwargs) + return _GLOBAL_CONFIG + + +@contextmanager +def config_context(new_config: DascoreConfig | None = None, **kwargs): + """ + Temporarily override the runtime config for the current thread/task. + + Parameters + ---------- + new_config + A complete [`DascoreConfig`](`dascore.config.DascoreConfig`) to install. + Mutually exclusive with keyword overrides. + **kwargs + Individual field overrides applied on top of the active config. + + Notes + ----- + The override is stored in a ``ContextVar``, so it is isolated per thread and + task and restored when the block exits. New OS threads do not inherit it + automatically; propagate it explicitly (e.g. capture + ``contextvars.copy_context()``), or rely on APIs that bind it for you such as + [`Spool.map`](`dascore.core.spool.BaseSpool.map`). + + Examples + -------- + >>> import dascore as dc + >>> with dc.config_context(debug=True): + ... assert dc.get_config().debug + >>> assert not dc.get_config().debug + """ + config = _build_config(get_config(), new_config, kwargs) + token = _CONFIG_OVERRIDE.set(config) + try: + yield config + finally: + _CONFIG_OVERRIDE.reset(token) def reset_config() -> DascoreConfig: - """Reset the active runtime config to defaults.""" - global _CONFIG - _CONFIG = DascoreConfig() - return _CONFIG + """Reset the process-wide runtime config base to defaults.""" + global _GLOBAL_CONFIG + _GLOBAL_CONFIG = DascoreConfig() + return _GLOBAL_CONFIG diff --git a/dascore/examples.py b/dascore/examples.py index 1eb6ba01b..b379bc90a 100644 --- a/dascore/examples.py +++ b/dascore/examples.py @@ -13,7 +13,7 @@ import dascore as dc import dascore.core from dascore.compat import random_state -from dascore.config import set_config +from dascore.config import config_context from dascore.exceptions import UnknownExampleError from dascore.utils.downloader import fetch from dascore.utils.imports import lazy_import @@ -29,7 +29,7 @@ def _load_example_patch_from_file(path: str | Path) -> dc.Patch: """Load the first patch from an example file without spool indirection.""" - with set_config(allow_dasdae_format_unpickle=True): + with config_context(allow_dasdae_format_unpickle=True): return dc.read(path)[0] diff --git a/dascore/io/dasdae/_compat.py b/dascore/io/dasdae/_compat.py index bd14fccbd..c24612a0c 100644 --- a/dascore/io/dasdae/_compat.py +++ b/dascore/io/dasdae/_compat.py @@ -64,7 +64,7 @@ def translate_legacy_attrs(attrs): "This DASDAE file contains legacy pickled coordinate metadata. " "Unpickling DASDAE format metadata is disabled by default for " "security. If you trust this file, enable legacy compatibility " - "with dc.set_config(allow_dasdae_format_unpickle=True)." + "with dc.config_context(allow_dasdae_format_unpickle=True)." ) raise InvalidFiberFileError(msg) with contextlib.suppress( diff --git a/dascore/utils/misc.py b/dascore/utils/misc.py index f0dfe0345..1ae72c99f 100644 --- a/dascore/utils/misc.py +++ b/dascore/utils/misc.py @@ -663,20 +663,28 @@ def wrapper(self, *args, **kwargs): class _MapFuncWrapper: """A class for unwrapping spools to base applies.""" - def __init__(self, func, kwargs, progress=True): + def __init__(self, func, kwargs, progress=True, config=None): self._func = func self._kwargs = kwargs self._progress = progress + # Bind the config active at map() call time so workers (threads or + # pickled into processes) apply the same config the caller had, rather + # than a fresh default or a scoped override that would not propagate. + self._config = config def __call__(self, spool): - iterable = spool - # in order to handle multiprocessing, we apply a secret tag of "_progress" - # to the first spool. This way only the first spool displays the - # the progress bar. A huge hack, maybe there is a better way? See #265. - if not getattr(spool, "_no_progress", False): - desc = f"Applying {self._func.__name__} to spool" - iterable = track(spool, desc) if self._progress else spool - return [self._func(x, **self._kwargs) for x in iterable] + from dascore.config import config_context + + with config_context(self._config): + iterable = spool + # in order to handle multiprocessing, we apply a secret tag of + # "_progress" to the first spool. This way only the first spool + # displays the progress bar. A huge hack, maybe there is a better + # way? See #265. + if not getattr(spool, "_no_progress", False): + desc = f"Applying {self._func.__name__} to spool" + iterable = track(spool, desc) if self._progress else spool + return [self._func(x, **self._kwargs) for x in iterable] def _spool_map(spool, func, size=None, client=None, progress=True, **kwargs): @@ -696,6 +704,8 @@ def _spool_map(spool, func, size=None, client=None, progress=True, **kwargs): **kwargs Keywords passed to func. """ + from dascore.config import get_config + # no client; simple for loop. desc = f"Applying {func.__name__} to spool" if client is None: @@ -711,7 +721,7 @@ def _spool_map(spool, func, size=None, client=None, progress=True, **kwargs): # displayed in one thread/process. for sub_spool in spools[1:]: sub_spool._no_progress = True - new_func = _MapFuncWrapper(func, kwargs, progress=progress) + new_func = _MapFuncWrapper(func, kwargs, progress=progress, config=get_config()) return [x for y in client.map(new_func, spools) for x in y] diff --git a/docs/changelog.qmd b/docs/changelog.qmd index 262631175..641dadf17 100644 --- a/docs/changelog.qmd +++ b/docs/changelog.qmd @@ -4,6 +4,7 @@ The [releases page](https://github.com/DASDAE/dascore/releases) tracks changes f ## Unreleased API Changes +- **Runtime configuration is now two-tier.** `dc.set_config(...)` sets the process-wide base permanently (visible from every thread and task) and returns the new config; it is no longer a context manager. Temporary, thread/task-local overrides use the new `dc.config_context(...)` context manager, which restores on exit and isolates concurrent overrides via a `ContextVar`. `Spool.map(...)` captures the config active at the call and re-applies it in each worker, so thread- and process-pool workers observe the caller's config (previously process workers saw defaults). Migrate `with dc.set_config(...)` to `with dc.config_context(...)`. - PRODML 2.1 now supports writing one raw time-by-distance patch with `dc.write` as a standalone HDF5 file. Full PRODML 2.3 conformance requires EPC/XML packaging, which DASCore does not write. - **The `dascore.io.sintela_binary` module is removed (no alias).** Both Sintela readers now live in `dascore.io.sintela`, which also provides the new protobuf reader; use `from dascore.io.sintela import SintelaBinaryV3`. Reading Sintela binary files through `dc.read`/`dc.spool`/`dc.scan` is unaffected — only the direct module import path changed. - **Removed the unused `PatchFileSummary` model** (`dascore.io.PatchFileSummary`), superseded by [`PatchSummary`](`dascore.PatchSummary`), along with the internal helpers `coord_summary_from_data` and `_normalize_coord_summary_dtype`. Build a coord first (`get_coord(...)`) and call `.to_summary()` to summarize raw array data. The unused `index_query_buffer` config option is also removed. diff --git a/docs/tutorial/configuration.qmd b/docs/tutorial/configuration.qmd index 5dc26ef44..e9220adbf 100644 --- a/docs/tutorial/configuration.qmd +++ b/docs/tutorial/configuration.qmd @@ -5,11 +5,11 @@ DASCore exposes a small runtime configuration surface through `dascore.config`. ```python from pathlib import Path -from dascore.config import get_config, set_config +from dascore.config import get_config, config_context print(get_config().remote_cache_dir) -with set_config(remote_cache_dir=Path("/tmp/dascore-remote-cache")): +with config_context(remote_cache_dir=Path("/tmp/dascore-remote-cache")): ... ``` @@ -21,9 +21,9 @@ History recording is also configurable: - `patch_history="disabled"` preserves any existing history but stops DASCore from appending new entries inside that config context. ```python -from dascore.config import set_config +from dascore.config import config_context -with set_config(patch_history="disabled"): +with config_context(patch_history="disabled"): ... ``` diff --git a/docs/tutorial/patch.qmd b/docs/tutorial/patch.qmd index df883e551..3ea33cdb2 100644 --- a/docs/tutorial/patch.qmd +++ b/docs/tutorial/patch.qmd @@ -251,15 +251,15 @@ filtered = patch.pass_filter(time=(1, 10)) print(filtered.attrs.history[-1]) ``` -If you want to suppress new history entries for a block of operations, use `set_config(patch_history="disabled")`. Existing history is preserved, but DASCore will stop appending new entries until that config context exits. +If you want to suppress new history entries for a block of operations, use `config_context(patch_history="disabled")`. Existing history is preserved, but DASCore will stop appending new entries until that config context exits. ```{python} -from dascore.config import set_config +from dascore.config import config_context patch = dc.get_example_patch().pass_filter(time=(1, 10)) old_history = patch.attrs.history -with set_config(patch_history="disabled"): +with config_context(patch_history="disabled"): out = patch.abs() assert out.attrs.history == old_history diff --git a/docs/tutorial/remote_patches.qmd b/docs/tutorial/remote_patches.qmd index 99f1e884e..62c01495d 100644 --- a/docs/tutorial/remote_patches.qmd +++ b/docs/tutorial/remote_patches.qmd @@ -45,7 +45,7 @@ The relevant runtime config values live in `dascore.config`. ```{python} from pathlib import Path -from dascore.config import get_config, set_config +from dascore.config import get_config, config_context config = get_config() @@ -54,7 +54,7 @@ print(config.allow_remote_cache) print(config.allow_remote_cache_for_metadata) print(config.warn_on_remote_cache) -with set_config( +with config_context( remote_cache_dir=Path("/tmp/dascore-remote-cache"), allow_remote_cache=True, allow_remote_cache_for_metadata=False, @@ -100,13 +100,13 @@ except RemoteCacheError as exc: If you know a metadata operation may need local caching, opt in explicitly. ```python -from dascore.config import set_config +from dascore.config import config_context from upath import UPath import dascore as dc http_path = UPath("http://example.com/data/prodml_2.1.h5") -with set_config(allow_remote_cache_for_metadata=True): +with config_context(allow_remote_cache_for_metadata=True): fmt = dc.get_format(http_path) summary = dc.scan(http_path)[0] payload = dc.scan_payloads(http_path, snap=False)[0] diff --git a/tests/conftest.py b/tests/conftest.py index 9369bd370..299a1580f 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -5,7 +5,7 @@ import os import shutil import warnings -from contextlib import suppress +from contextlib import contextmanager, suppress from pathlib import Path import h5py @@ -17,7 +17,7 @@ import dascore as dc import dascore.examples as ex from dascore.compat import random_state -from dascore.config import set_config +from dascore.config import get_config, set_config from dascore.constants import SpoolType from dascore.core import Patch from dascore.core.spool import Spool @@ -87,17 +87,33 @@ def pytest_sessionstart(session): # Test-time debug defaults are applied by fixture to avoid state leakage. +@contextmanager +def _permanent_config(**overrides): + """Set process-wide config for the block, restoring the prior config. + + Test fixtures use the permanent base (not a scoped ``config_context``) so + overrides are visible to worker threads and forked processes the tests + spawn, and so they do not shadow a test's own ``set_config`` calls. + """ + previous = get_config() + set_config(**overrides) + try: + yield + finally: + set_config(previous) + + @pytest.fixture(autouse=True) def use_test_config(): """Run tests with debug mode enabled unless overridden locally.""" - with set_config(debug=True): + with _permanent_config(debug=True): yield @pytest.fixture(scope="session", autouse=True) def allow_legacy_dasdae_coord_unpickle(): """Test fixtures may rely on trusted historical DASDAE coord payloads.""" - with set_config(allow_dasdae_format_unpickle=True): + with _permanent_config(allow_dasdae_format_unpickle=True): yield @@ -105,7 +121,7 @@ def allow_legacy_dasdae_coord_unpickle(): def swap_index_map_path(tmp_path_factory): """For all tests cases, use a temporary index-map directory.""" tmp_map_dir = tmp_path_factory.mktemp("cache_paths") / "path_map" - with set_config(directory_index_map_dir=tmp_map_dir): + with _permanent_config(directory_index_map_dir=tmp_map_dir): yield diff --git a/tests/test_io/test_dasdae/test_dasdae.py b/tests/test_io/test_dasdae/test_dasdae.py index ae5f5a379..e0dc41a4b 100644 --- a/tests/test_io/test_dasdae/test_dasdae.py +++ b/tests/test_io/test_dasdae/test_dasdae.py @@ -14,7 +14,7 @@ import dascore as dc from dascore.compat import random_state -from dascore.config import set_config +from dascore.config import config_context from dascore.core.coords import CoordString from dascore.exceptions import InvalidFiberFileError from dascore.io import dasdae as dasdae_mod @@ -159,7 +159,7 @@ def test_round_trip_empty_patch(self, written_dascore_v1_empty): def test_reads_legacy_fixture(self): """Legacy DASDAE fixtures still need to remain readable.""" path = fetch("example_dasdae_event_1.h5") - with set_config(allow_dasdae_format_unpickle=True): + with config_context(allow_dasdae_format_unpickle=True): spool = dc.read(path, file_format="DASDAE") assert len(spool) == 1 assert spool[0].dims @@ -372,14 +372,14 @@ def to_summary(self): def test_translate_legacy_attrs_ignores_non_mapping_coords(self): """Undecodable string coord payloads are ignored once opted in.""" - with set_config(allow_dasdae_format_unpickle=True): + with config_context(allow_dasdae_format_unpickle=True): out = translate_legacy_attrs({"coords": "pickled-coords-placeholder"}) assert "coords" not in out def test_translate_legacy_attrs_decodes_pickled_coord_payload(self): """Legacy pickled coord payloads should still restore coord metadata.""" payload = pickle.dumps({"distance": {"min": 0, "max": 1, "units": "m"}}) - with set_config(allow_dasdae_format_unpickle=True): + with config_context(allow_dasdae_format_unpickle=True): out = translate_legacy_attrs({"coords": payload.decode("latin1")}) assert out["distance_units"] == "m" assert out["distance_min"] == 0 @@ -394,20 +394,20 @@ def __reduce__(self): return (executed.append, ("pickle ran",)) payload = pickle.dumps(_Payload()).decode("latin1") - with set_config(allow_dasdae_format_unpickle=False): + with config_context(allow_dasdae_format_unpickle=False): with pytest.raises(InvalidFiberFileError, match="unpickle"): translate_legacy_attrs({"coords": payload}) assert not executed, "pickle.loads ran before the security gate" def test_scan_preserves_legacy_coord_units_from_attr_payload(self): """Legacy attr coord units should backfill missing coord-node units.""" - with set_config(allow_dasdae_format_unpickle=True): + with config_context(allow_dasdae_format_unpickle=True): summary = dc.scan(fetch("UoU_lf_urban.hdf5"))[0] assert str(summary.coords["distance"].units) == "1 m" def test_read_legacy_coord_payload_requires_opt_in(self): """Legacy pickled coord metadata should fail closed by default.""" - with set_config(allow_dasdae_format_unpickle=False): + with config_context(allow_dasdae_format_unpickle=False): with pytest.raises( InvalidFiberFileError, match="allow_dasdae_format_unpickle=True" ): diff --git a/tests/test_io/test_index/test_index_edge_cases.py b/tests/test_io/test_index/test_index_edge_cases.py index 0fc9eb2ea..aacf83b0b 100644 --- a/tests/test_io/test_index/test_index_edge_cases.py +++ b/tests/test_io/test_index/test_index_edge_cases.py @@ -1225,7 +1225,7 @@ def test_legacy_entry_ignored(self, tmp_path): with h5py.File(legacy, "w") as fh: fh.create_dataset("x", data=[1, 2, 3]) map_dir = tmp_path / "path_map" - with dc.set_config(directory_index_map_dir=map_dir): + with dc.config_context(directory_index_map_dir=map_dir): _set_mapped_index_path(data_dir, legacy, map_dir) spool = dc.spool(data_dir).update() assert len(spool) == 1 diff --git a/tests/test_io/test_index/test_plan.py b/tests/test_io/test_index/test_plan.py index a8db642a3..451552f17 100644 --- a/tests/test_io/test_index/test_plan.py +++ b/tests/test_io/test_index/test_plan.py @@ -136,7 +136,7 @@ def test_gap_splits_partition(self): def test_plan_records_params(self, random_flat): """Plans record resolved parameters, not config references.""" - with dc.set_config(sampling_group_tolerance=0.02): + with dc.config_context(sampling_group_tolerance=0.02): plan = build_chunk_plan(random_flat, time=None) assert plan.params["sampling_group_tolerance"] == 0.02 assert isinstance(plan.params["group"], tuple) @@ -282,7 +282,7 @@ def test_group_override(self, two_station_flat): def test_config_group(self, two_station_flat): """Config groupby_attrs drives the default partitioning.""" - with dc.set_config(groupby_attrs=("network",)): + with dc.config_context(groupby_attrs=("network",)): with pytest.raises(CoordMergeError, match="station"): build_chunk_plan(two_station_flat, time=None) diff --git a/tests/test_io/test_indexer.py b/tests/test_io/test_indexer.py index 79ad670ad..171fd9a01 100644 --- a/tests/test_io/test_indexer.py +++ b/tests/test_io/test_indexer.py @@ -13,7 +13,7 @@ import pytest from upath import UPath -from dascore.config import set_config +from dascore.config import config_context from dascore.exceptions import InvalidSpoolError from dascore.io.index.indexer import DBDirectoryIndexer from dascore.utils.patch import get_patch_names @@ -79,7 +79,7 @@ def test_read_only_index_name_is_stable(self, unwritable_directory, tmp_path): from dascore.io.index.indexer import _path_digest map_dir = tmp_path / "path_map" - with set_config(directory_index_map_dir=map_dir): + with config_context(directory_index_map_dir=map_dir): first = DBDirectoryIndexer(unwritable_directory).index_path digest = _path_digest(unwritable_directory) assert first.name == f"_dascore_index_{digest}.sqlite3" @@ -118,7 +118,7 @@ def test_corrupt_cache(self, tmp_path): entry = _map_entry_path(data_dir, map_dir) entry.parent.mkdir(parents=True) entry.write_text("{'bad': 'json'") - with set_config(directory_index_map_dir=map_dir): + with config_context(directory_index_map_dir=map_dir): indexer = DBDirectoryIndexer(data_dir) # The corrupt entry reads as a miss, so the writable data dir keeps # its in-directory index rather than crashing. @@ -140,7 +140,7 @@ def test_local_upath_normalized_to_path(self, tmp_path): def test_index_map_dir_comes_from_config(self, tmp_path): """Index map dir should be sourced from runtime configuration.""" index_map_dir = tmp_path / "path_map" - with set_config(directory_index_map_dir=index_map_dir): + with config_context(directory_index_map_dir=index_map_dir): out = DBDirectoryIndexer(tmp_path) assert out.index_map_dir == index_map_dir diff --git a/tests/test_io/test_io_core.py b/tests/test_io/test_io_core.py index f1a65af6c..b892ee3d3 100644 --- a/tests/test_io/test_io_core.py +++ b/tests/test_io/test_io_core.py @@ -15,7 +15,7 @@ from upath import UPath import dascore as dc -from dascore.config import set_config +from dascore.config import config_context from dascore.constants import SpoolType from dascore.exceptions import ( InvalidFiberIOError, @@ -1171,7 +1171,7 @@ def track(self, *args, **kwargs): # Switch off debug to force progress bar, then make contents to scan. contents = list(dc.examples.get_example_spool(length=22)) - with set_config(debug=False): + with config_context(debug=False): with pytest.raises(KeyboardInterrupt, match="test interrupt"): dc.scan(contents, progress=Progress()) diff --git a/tests/test_io/test_remote_common_io.py b/tests/test_io/test_remote_common_io.py index bf9ceff22..7ec863b26 100644 --- a/tests/test_io/test_remote_common_io.py +++ b/tests/test_io/test_remote_common_io.py @@ -7,7 +7,7 @@ import pytest import dascore as dc -from dascore.config import set_config +from dascore.config import get_config, set_config from dascore.utils.misc import suppress_warnings from tests.test_io._common_io_test_utils import ( get_flat_io_test, @@ -62,12 +62,21 @@ def suppress_expected_remote_cache_warnings(): @pytest.fixture(scope="module", autouse=True) def isolated_remote_cache(tmp_path_factory): - """Keep the common remote matrix in its own cache root.""" - with set_config( + """Keep the common remote matrix in its own cache root. + + Uses the permanent config base (not a scoped ``config_context``) because a + module-scoped fixture spans many tests; the scoped override belongs to a + single call block, not a fixture that stays open across the module. + """ + previous = get_config() + set_config( remote_cache_dir=tmp_path_factory.mktemp("remote_common_cache"), allow_remote_cache_for_metadata=True, - ): + ) + try: yield + finally: + set_config(previous) def _get_remote_case(fetch_name: str, to_http_range_path): diff --git a/tests/test_io/test_remote_http.py b/tests/test_io/test_remote_http.py index 2433e131a..3c3f82e0f 100644 --- a/tests/test_io/test_remote_http.py +++ b/tests/test_io/test_remote_http.py @@ -9,7 +9,7 @@ from upath import UPath import dascore as dc -from dascore.config import set_config +from dascore.config import config_context from dascore.exceptions import InvalidSpoolError, RemoteCacheError from dascore.utils.misc import suppress_warnings from dascore.utils.remote_io import clear_remote_file_cache, get_remote_cache_path @@ -48,7 +48,7 @@ def suppress_expected_remote_cache_warnings(request): @pytest.fixture(autouse=True) def isolated_remote_cache(tmp_path): """Use one isolated remote cache per test to avoid suite-order slowdowns.""" - with set_config(remote_cache_dir=tmp_path / "remote_cache"): + with config_context(remote_cache_dir=tmp_path / "remote_cache"): clear_remote_file_cache() yield clear_remote_file_cache() @@ -137,7 +137,7 @@ def test_http_hdf5_fallback_warns_once_and_reuses_cached_local_copy( fname = "prodml_2.1.h5" ensure_http_regression_file(fname) path = http_regression_das_path / fname - with set_config( + with config_context( allow_remote_cache_for_metadata=True, warn_on_remote_cache=True ): with pytest.warns(UserWarning, match="Downloading remote file"): diff --git a/tests/test_io/test_remote_memory.py b/tests/test_io/test_remote_memory.py index 4c453bacb..b3d07cd1e 100644 --- a/tests/test_io/test_remote_memory.py +++ b/tests/test_io/test_remote_memory.py @@ -8,7 +8,7 @@ from upath import UPath import dascore as dc -from dascore.config import set_config +from dascore.config import config_context from dascore.exceptions import InvalidSpoolError from dascore.utils.downloader import fetch from dascore.utils.remote_io import clear_remote_file_cache, get_remote_cache_path @@ -44,7 +44,7 @@ def _copy(fetch_name: str, namespace: str) -> tuple[Path, UPath]: @pytest.fixture(autouse=True) def isolated_remote_cache(tmp_path): """Use one isolated remote cache per test to avoid cross-test cleanup cost.""" - with set_config(remote_cache_dir=tmp_path / "remote_cache"): + with config_context(remote_cache_dir=tmp_path / "remote_cache"): clear_remote_file_cache() yield clear_remote_file_cache() diff --git a/tests/test_proc/test_rolling.py b/tests/test_proc/test_rolling.py index bf4fd7954..d33d99dc9 100644 --- a/tests/test_proc/test_rolling.py +++ b/tests/test_proc/test_rolling.py @@ -8,7 +8,7 @@ import dascore as dc import dascore.proc.coords -from dascore.config import set_config +from dascore.config import config_context from dascore.exceptions import ParameterError from dascore.units import m from dascore.utils.misc import all_close @@ -215,7 +215,7 @@ def test_history_disabled(self, random_patch): """Rolling history should not append when patch history is disabled.""" time_step = random_patch.get_coord("time").step expected = random_patch.attrs.history - with set_config(patch_history="disabled"): + with config_context(patch_history="disabled"): out = random_patch.rolling(time=4 * time_step).mean() assert out.attrs.history == expected diff --git a/tests/test_utils/test_config.py b/tests/test_utils/test_config.py index ccf48e725..7a4a3f3c5 100644 --- a/tests/test_utils/test_config.py +++ b/tests/test_utils/test_config.py @@ -2,31 +2,55 @@ from __future__ import annotations +import threading + import pytest +import dascore as dc from dascore.config import ( DascoreConfig, config_attr, + config_context, get_config, reset_config, set_config, ) +def _worker_read_float_precision(patch): + """Return the runtime float precision a worker task observes.""" + return dc.get_config().display_float_precision + + class TestSetConfig: - """Tests for updating runtime configuration.""" + """Tests for the permanent, process-wide config base.""" + + def setup_method(self): + """Remember the active config so each test restores it exactly.""" + self._saved = get_config() def teardown_method(self): - """Reset global config after each test.""" - reset_config() + """Restore the pre-test config (not hard defaults, which would drop + session-level overrides such as the temporary index-map dir). + """ + set_config(self._saved) def test_accepts_direct_config_instance(self): - """Passing a validated config should install it.""" - previous = get_config() + """Passing a validated config installs it permanently.""" new = DascoreConfig(debug=True) - with set_config(new): - assert get_config().debug is True - assert get_config() == previous + set_config(new) + assert get_config().debug is True + + def test_persists_until_reset(self): + """A permanent set stays active (it is not auto-restored).""" + set_config(display_float_precision=5) + assert get_config().display_float_precision == 5 + + def test_reset_config_returns_to_defaults(self): + """reset_config restores the process-wide base to defaults.""" + set_config(display_float_precision=5) + reset_config() + assert get_config().display_float_precision == 3 def test_invalid_new_config_raises(self): """Arbitrary objects should not be accepted as configs.""" @@ -38,10 +62,63 @@ def test_new_config_and_kwargs_raise(self): with pytest.raises(ValueError, match="new_config"): set_config(DascoreConfig(), debug=True) + def test_invalid_patch_history_raises(self): + """Unsupported patch history policies should be rejected.""" + with pytest.raises(ValueError, match="patch_history"): + set_config(patch_history="verbose-ish") + + def test_sampling_group_tolerance_must_be_positive(self): + """Non-positive tolerances are rejected.""" + with pytest.raises(ValueError, match="sampling_group_tolerance"): + set_config(sampling_group_tolerance=0) + + def test_visible_from_new_thread(self): + """A permanent set is visible from threads launched afterward.""" + set_config(display_float_precision=9) + seen = {} + + def worker(): + seen["value"] = get_config().display_float_precision + + thread = threading.Thread(target=worker) + thread.start() + thread.join() + assert seen["value"] == 9 + + +class TestConfigContext: + """Tests for scoped, thread/task-local config overrides.""" + + def test_scoped_override_restored_on_exit(self): + """A scoped override applies inside the block and restores after.""" + previous = get_config() + with config_context(display_float_precision=6): + assert get_config().display_float_precision == 6 + assert get_config() == previous + + def test_accepts_direct_config_instance(self): + """A full config can be installed for the scope of a block.""" + previous = get_config() + new = DascoreConfig(display_float_precision=6) + with config_context(new): + assert get_config().display_float_precision == 6 + assert get_config() == previous + + def test_overrides_stack(self): + """A nested override builds on the enclosing one and reverts alone.""" + with config_context(display_float_precision=6): + with config_context(display_array_threshold=7): + config = get_config() + assert config.display_array_threshold == 7 + assert config.display_float_precision == 6 + # The inner override reverts; the outer one remains. + assert get_config().display_array_threshold == 100 + assert get_config().display_float_precision == 6 + def test_remote_cache_controls_can_be_overridden(self): - """Remote cache policy config should round-trip through set_config.""" + """Remote cache policy config round-trips through config_context.""" previous = get_config() - with set_config( + with config_context( allow_remote_cache=False, allow_remote_cache_for_metadata=True, warn_on_remote_cache=False, @@ -54,28 +131,52 @@ def test_remote_cache_controls_can_be_overridden(self): assert config.allow_dasdae_format_unpickle is True assert get_config() == previous - def test_patch_history_can_be_disabled(self): - """Patch history policy should round-trip through set_config.""" - previous = get_config() - with set_config(patch_history="disabled"): - assert get_config().patch_history == "disabled" - assert get_config() == previous + def test_groupby_attrs_coerced_to_tuple(self): + """List inputs coerce to the immutable tuple form.""" + with config_context(groupby_attrs=["tag"]): + assert get_config().groupby_attrs == ("tag",) - def test_invalid_patch_history_raises(self): - """Unsupported patch history policies should be rejected.""" - with pytest.raises(ValueError, match="patch_history"): - set_config(patch_history="verbose-ish") + def test_invalid_new_config_raises(self): + """Validation is shared with set_config.""" + with pytest.raises(TypeError, match="DascoreConfig"): + with config_context(object()): + pass def test_config_attr_reflects_runtime_config(self): - """Config descriptors should resolve against the active runtime config.""" + """Config descriptors resolve against the active runtime config.""" class _UsesConfig: value = config_attr("display_float_precision") assert _UsesConfig().value == get_config().display_float_precision - with set_config(display_float_precision=7): + with config_context(display_float_precision=7): assert _UsesConfig().value == 7 + def test_concurrent_overrides_are_isolated(self): + """Two threads with different scoped overrides never stomp each other.""" + barrier = threading.Barrier(2) + results = {} + + def worker(name, value): + with config_context(display_float_precision=value): + # Force overlap: both threads sit inside their context at once. + barrier.wait() + results[name] = get_config().display_float_precision + + threads = [ + threading.Thread(target=worker, args=("a", 1)), + threading.Thread(target=worker, args=("b", 2)), + ] + for thread in threads: + thread.start() + for thread in threads: + thread.join() + assert results == {"a": 1, "b": 2} + + +class TestConfigDefaults: + """Sanity checks on default field values.""" + def test_groupby_attrs_default(self): """The default group attrs are the conventional identity set.""" expected = ( @@ -89,30 +190,30 @@ def test_groupby_attrs_default(self): ) assert get_config().groupby_attrs == expected - def test_groupby_attrs_override(self): - """groupby_attrs round-trips through scoped set_config.""" - previous = get_config() - with set_config(groupby_attrs=("network", "station")): - assert get_config().groupby_attrs == ("network", "station") - assert get_config() == previous - - def test_groupby_attrs_coerced_to_tuple(self): - """List inputs coerce to the immutable tuple form.""" - with set_config(groupby_attrs=["tag"]): - assert get_config().groupby_attrs == ("tag",) - def test_sampling_group_tolerance_default(self): """The default sampling group tolerance is 5%.""" assert get_config().sampling_group_tolerance == 0.05 - def test_sampling_group_tolerance_override(self): - """sampling_group_tolerance round-trips through scoped set_config.""" - previous = get_config() - with set_config(sampling_group_tolerance=0.01): - assert get_config().sampling_group_tolerance == 0.01 - assert get_config() == previous - def test_sampling_group_tolerance_must_be_positive(self): - """Non-positive tolerances are rejected.""" - with pytest.raises(ValueError, match="sampling_group_tolerance"): - set_config(sampling_group_tolerance=0) +class TestMapConfigBinding: + """Spool.map applies the config active when map() was called.""" + + def test_thread_pool_sees_call_time_config(self): + """A thread-pool map sees the caller's scoped override.""" + from concurrent.futures import ThreadPoolExecutor + + spool = dc.get_example_spool() + with config_context(display_float_precision=8): + with ThreadPoolExecutor(2) as executor: + out = spool.map(_worker_read_float_precision, client=executor) + assert set(out) == {8} + + def test_process_pool_sees_call_time_config(self): + """A process-pool map sees the caller's scoped override too.""" + from concurrent.futures import ProcessPoolExecutor + + spool = dc.get_example_spool() + with config_context(display_float_precision=8): + with ProcessPoolExecutor(2) as executor: + out = spool.map(_worker_read_float_precision, client=executor) + assert set(out) == {8} diff --git a/tests/test_utils/test_display.py b/tests/test_utils/test_display.py index 1e9e4248f..06e35d862 100644 --- a/tests/test_utils/test_display.py +++ b/tests/test_utils/test_display.py @@ -6,7 +6,7 @@ import pandas as pd import dascore as dc -from dascore.config import set_config +from dascore.config import config_context from dascore.utils.display import array_to_text, get_nice_text from dascore.utils.patch import _format_values @@ -41,7 +41,7 @@ def test_timestamp(self): def test_float_precision_config(self): """Float display precision should come from runtime config.""" - with set_config(display_float_precision=1): + with config_context(display_float_precision=1): txt = get_nice_text(1.234) assert str(txt) == "1.2" @@ -52,14 +52,14 @@ class TestArrayFormatting: def test_array_threshold_config(self): """Array display truncation threshold should be configurable.""" data = np.arange(10) - with set_config(display_array_threshold=3): + with config_context(display_array_threshold=3): txt = array_to_text(data) assert "..." in str(txt) def test_patch_history_threshold_config(self): """Patch history formatting should use the configured threshold.""" data = np.arange(10) - with set_config( + with config_context( display_float_precision=0, display_patch_history_array_threshold=3, ): diff --git a/tests/test_utils/test_downloader.py b/tests/test_utils/test_downloader.py index d63456a63..d0c3d29ed 100644 --- a/tests/test_utils/test_downloader.py +++ b/tests/test_utils/test_downloader.py @@ -5,7 +5,7 @@ import pandas as pd import pytest -from dascore.config import set_config +from dascore.config import config_context from dascore.constants import DATA_VERSION from dascore.utils.downloader import ( LARGE_REGISTRY_FILES, @@ -59,7 +59,7 @@ def test_existing_file(self, registry_df): def test_fetcher_path_comes_from_config(self, tmp_path): """Downloader fetchers should honor the configured cache directory.""" cache_dir = tmp_path / "downloads" - with set_config(downloader_cache_dir=cache_dir): + with config_context(downloader_cache_dir=cache_dir): active_fetcher = get_fetcher() assert fetcher.path == active_fetcher.path assert active_fetcher.path.parent == cache_dir @@ -105,6 +105,6 @@ def test_cache_key_includes_expected_parts(self): def test_cache_info_respects_configured_cache_dir(self, tmp_path): """Cache info should reflect the configured downloader cache root.""" cache_dir = tmp_path / "downloads" - with set_config(downloader_cache_dir=cache_dir): + with config_context(downloader_cache_dir=cache_dir): info = get_test_data_cache_info() assert info.cache_path == cache_dir diff --git a/tests/test_utils/test_io_utils.py b/tests/test_utils/test_io_utils.py index 096ae4d17..c241c9061 100644 --- a/tests/test_utils/test_io_utils.py +++ b/tests/test_utils/test_io_utils.py @@ -12,7 +12,7 @@ import dascore as dc import dascore.utils.remote_io as remote_io -from dascore.config import set_config +from dascore.config import config_context from dascore.exceptions import PatchConversionError, RemoteCacheError from dascore.utils.hdf5 import ( H5Reader, @@ -401,7 +401,7 @@ def _open(_self, _mode, **kwargs): "constructor", staticmethod(lambda *args, **kwargs: object()), ) - with set_config(remote_hdf5_block_size=1234): + with config_context(remote_hdf5_block_size=1234): H5Reader.get_handle(path) assert opened["block_size"] == 1234 assert opened["cache_type"] == "readahead" @@ -455,7 +455,7 @@ class TestIOResourceManager: @pytest.fixture(autouse=True) def clear_remote_cache(self): """Ensure remote cache state doesn't leak between tests.""" - with set_config(warn_on_remote_cache=False): + with config_context(warn_on_remote_cache=False): clear_remote_file_cache() yield clear_remote_file_cache() @@ -532,7 +532,7 @@ def test_remote_cache_dir_comes_from_config(self, tmp_path): path = UPath("memory://dascore/io_resource_test_custom_cache.txt") path.write_text("hello") cache_dir = tmp_path / "remote-cache" - with set_config(remote_cache_dir=cache_dir): + with config_context(remote_cache_dir=cache_dir): local_path = ensure_local_file(path) assert cache_dir in local_path.parents assert local_path.exists() @@ -631,9 +631,9 @@ def test_ensure_local_file_respects_cache_dir_changes(self, tmp_path): first_cache = tmp_path / "remote-cache-a" second_cache = tmp_path / "remote-cache-b" - with set_config(remote_cache_dir=first_cache): + with config_context(remote_cache_dir=first_cache): first = ensure_local_file(path) - with set_config(remote_cache_dir=second_cache): + with config_context(remote_cache_dir=second_cache): second = ensure_local_file(path) assert first_cache in first.parents @@ -684,7 +684,7 @@ def __exit__(self, *_args): path = UPath("memory://dascore/io_resource_test_block_size.bin") monkeypatch.setattr(type(path), "open", lambda *_args, **_kwargs: handle) - with set_config(remote_download_block_size=321): + with config_context(remote_download_block_size=321): local_path = ensure_local_file(path) assert local_path.exists() @@ -726,7 +726,7 @@ def __exit__(self, *_args): resource = _HTTPResource() monkeypatch.setattr(remote_io, "coerce_to_upath", lambda resource: resource) - with set_config(remote_download_block_size=2): + with config_context(remote_download_block_size=2): local_path = tmp_path / "downloaded.bin" remote_io._download_remote_file(resource, local_path) @@ -759,7 +759,7 @@ def test_ensure_local_file_warns_on_first_remote_download(self): """First-time remote cache materialization should warn.""" path = UPath("memory://dascore/io_resource_test_warn.txt") path.write_text("hello") - with set_config(warn_on_remote_cache=True): + with config_context(warn_on_remote_cache=True): with pytest.warns( UserWarning, match=r"Downloading remote file memory://\.\.\./io_resource_test_warn\.txt", @@ -771,7 +771,7 @@ def test_ensure_local_file_reuse_is_silent_after_first_download(self): """Cache hits should not warn after a remote file is already cached.""" path = UPath("memory://dascore/io_resource_test_warn_reuse.txt") path.write_text("hello") - with set_config(warn_on_remote_cache=True): + with config_context(warn_on_remote_cache=True): with pytest.warns(UserWarning, match="Downloading remote file"): first = ensure_local_file(path) with suppress_warnings(action="always", record=True) as record: @@ -783,7 +783,7 @@ def test_ensure_local_file_warning_can_be_disabled(self): """Configured warning suppression should keep downloads silent.""" path = UPath("memory://dascore/io_resource_test_warn_off.txt") path.write_text("hello") - with set_config(warn_on_remote_cache=False): + with config_context(warn_on_remote_cache=False): with suppress_warnings(action="always", record=True) as record: local_path = ensure_local_file(path) assert not record @@ -805,7 +805,7 @@ def test_ensure_local_file_raises_when_remote_cache_disabled(self): """Disabling remote caching should block local materialization.""" path = UPath("memory://dascore/io_resource_test_disabled.txt") path.write_text("hello") - with set_config(allow_remote_cache=False): + with config_context(allow_remote_cache=False): with pytest.raises(RemoteCacheError, match="Remote caching is disabled"): ensure_local_file(path) assert not list(get_remote_cache_path().rglob(path.name)) @@ -825,7 +825,7 @@ def test_metadata_scope_allows_download_when_enabled(self): """Metadata scope should permit downloads when opted in.""" path = UPath("memory://dascore/io_resource_test_metadata_enabled.txt") path.write_text("hello") - with set_config( + with config_context( allow_remote_cache_for_metadata=True, warn_on_remote_cache=False ): with remote_cache_scope("metadata"): @@ -1008,7 +1008,7 @@ def test_h5_reader_warns_when_no_range_fallback_downloads(self, monkeypatch): staticmethod(lambda handle, **_kwargs: handle.seek(1) or object()), ) - with set_config(warn_on_remote_cache=True): + with config_context(warn_on_remote_cache=True): with pytest.warns(UserWarning, match="Downloading remote file"): H5Reader.get_handle(path) @@ -1033,7 +1033,7 @@ def test_h5_reader_raises_when_no_range_fallback_cache_disabled(self, monkeypatc staticmethod(lambda handle, **_kwargs: handle.seek(1) or object()), ) - with set_config(allow_remote_cache=False): + with config_context(allow_remote_cache=False): with pytest.raises(RemoteCacheError, match="Remote caching is disabled"): H5Reader.get_handle(path) diff --git a/tests/test_utils/test_patch_utils.py b/tests/test_utils/test_patch_utils.py index b19f0ece5..ec859be66 100644 --- a/tests/test_utils/test_patch_utils.py +++ b/tests/test_utils/test_patch_utils.py @@ -12,7 +12,7 @@ import dascore as dc from dascore import patch_function -from dascore.config import set_config +from dascore.config import config_context from dascore.constants import PatchType from dascore.exceptions import ( CoordError, @@ -298,7 +298,7 @@ def func_with_patch_arg(patch, other_patch): def test_history_disabled_for_patch_function(self, random_patch): """Decorator-based history should respect disabled config.""" - with set_config(patch_history="disabled"): + with config_context(patch_history="disabled"): out = add_one(random_patch) assert out.attrs.history == random_patch.attrs.history @@ -306,20 +306,20 @@ def test_disabled_preserves_existing_history(self, random_patch): """Disabling history should stop new entries without clearing old ones.""" patch = random_patch.pass_filter(time=(10, 20)) original_history = patch.attrs.history - with set_config(patch_history="disabled"): + with config_context(patch_history="disabled"): out = add_one(patch) assert out.attrs.history == original_history def test_concatenate_history_disabled(self, random_patch): """Concatenate should use the shared history helper.""" - with set_config(patch_history="disabled"): + with config_context(patch_history="disabled"): out = concatenate_patches([random_patch, random_patch], time=None) assert len(out) == 1 assert out[0].attrs.history == random_patch.attrs.history def test_stack_history_disabled(self, random_patch): """Stack should use the shared history helper.""" - with set_config(patch_history="disabled"): + with config_context(patch_history="disabled"): out = stack_patches([random_patch, random_patch]) assert out.attrs.history == random_patch.attrs.history diff --git a/tests/test_utils/test_progress.py b/tests/test_utils/test_progress.py index a9df7de77..ddc98e01c 100644 --- a/tests/test_utils/test_progress.py +++ b/tests/test_utils/test_progress.py @@ -4,7 +4,7 @@ from rich.progress import Progress -from dascore.config import set_config +from dascore.config import config_context from dascore.utils.progress import get_progress_instance, track @@ -13,7 +13,7 @@ class TestProgressBar: def test_progressbar_shows(self): """Undo debug patch to progress bar shows.""" - with set_config(debug=False): + with config_context(debug=False): for _ in track([1, 2, 3], "testing_tracker"): pass @@ -31,6 +31,6 @@ def __init__(self, *_args, **kwargs): seen.update(kwargs) monkeypatch.setattr("dascore.utils.progress.Progress", DummyProgress) - with set_config(progress_basic_refresh_per_second=0.5): + with config_context(progress_basic_refresh_per_second=0.5): get_progress_instance("basic") assert seen["refresh_per_second"] == 0.5 From 9e601e25132056049bc2e1e0a02750f702c0b3a6 Mon Sep 17 00:00:00 2001 From: Derrick Chambers Date: Fri, 24 Jul 2026 15:08:20 +0200 Subject: [PATCH 2/3] Address Codex review: lock global config, fix benchmark fixture, docs, wrapper - set_config/reset_config now serialize the global read-modify-write under a module lock so concurrent updates cannot lose fields or return another thread's config. - benchmarks/test_io_benchmarks.py module fixture uses permanent save/set/ restore instead of `with set_config(...)` (no longer a context manager). - config_context docstring: thread inheritance is runtime-dependent (sys.flags.thread_inherit_context), and an inherited copy is not undone on block exit. - _MapFuncWrapper takes config as a required arg (its only caller always supplies it), strengthening the call-time binding invariant. --- benchmarks/test_io_benchmarks.py | 14 +++++++++++--- dascore/config.py | 24 +++++++++++++++++------- dascore/utils/misc.py | 4 ++-- 3 files changed, 30 insertions(+), 12 deletions(-) diff --git a/benchmarks/test_io_benchmarks.py b/benchmarks/test_io_benchmarks.py index 6feda6641..c5f8c9dcd 100644 --- a/benchmarks/test_io_benchmarks.py +++ b/benchmarks/test_io_benchmarks.py @@ -8,7 +8,7 @@ import pytest import dascore as dc -from dascore.config import set_config +from dascore.config import get_config, set_config from dascore.exceptions import DependencyError from dascore.utils.downloader import fetch, get_registry_df @@ -31,9 +31,17 @@ def test_file_paths(): @pytest.fixture(scope="module", autouse=True) def allow_legacy_dasdae_coord_unpickle(): - """Benchmarks include trusted historical DASDAE fixtures from the registry.""" - with set_config(allow_dasdae_format_unpickle=True): + """Benchmarks include trusted historical DASDAE fixtures from the registry. + + Uses the permanent config base (not a scoped ``config_context``) because a + module-scoped fixture spans many benchmarks. + """ + previous = get_config() + set_config(allow_dasdae_format_unpickle=True) + try: yield + finally: + set_config(previous) class TestIOBenchmarks: diff --git a/dascore/config.py b/dascore/config.py index d93b18a8b..262199ad8 100644 --- a/dascore/config.py +++ b/dascore/config.py @@ -6,6 +6,7 @@ from contextvars import ContextVar from pathlib import Path from tempfile import gettempdir +from threading import Lock from typing import Literal import pooch @@ -145,6 +146,7 @@ def _coerce_path(cls, value): # overrides from `config_context(...)` live in a ContextVar so concurrent # blocks stay isolated per thread/task and never clobber one another. _GLOBAL_CONFIG: DascoreConfig = DascoreConfig() +_GLOBAL_CONFIG_LOCK = Lock() _CONFIG_OVERRIDE: ContextVar[DascoreConfig | None] = ContextVar( "dascore_config_override", default=None ) @@ -218,8 +220,11 @@ def set_config(new_config: DascoreConfig | None = None, **kwargs) -> DascoreConf >>> _ = dc.reset_config() """ global _GLOBAL_CONFIG - _GLOBAL_CONFIG = _build_config(_GLOBAL_CONFIG, new_config, kwargs) - return _GLOBAL_CONFIG + # Serialize the read-modify-write so concurrent keyword updates cannot lose + # each other's fields or return a config another thread just installed. + with _GLOBAL_CONFIG_LOCK: + _GLOBAL_CONFIG = _build_config(_GLOBAL_CONFIG, new_config, kwargs) + return _GLOBAL_CONFIG @contextmanager @@ -238,9 +243,13 @@ def config_context(new_config: DascoreConfig | None = None, **kwargs): Notes ----- The override is stored in a ``ContextVar``, so it is isolated per thread and - task and restored when the block exits. New OS threads do not inherit it - automatically; propagate it explicitly (e.g. capture - ``contextvars.copy_context()``), or rely on APIs that bind it for you such as + task and restored when the block exits. Whether a newly started OS thread + inherits a copy of the override is runtime-dependent + (``sys.flags.thread_inherit_context`` -- normally enabled on free-threaded + builds and disabled otherwise); an inherited copy is not undone by this + block's exit. For deterministic propagation, capture + ``contextvars.copy_context()`` and run the worker with it, or rely on APIs + that bind the config for you such as [`Spool.map`](`dascore.core.spool.BaseSpool.map`). Examples @@ -261,5 +270,6 @@ def config_context(new_config: DascoreConfig | None = None, **kwargs): def reset_config() -> DascoreConfig: """Reset the process-wide runtime config base to defaults.""" global _GLOBAL_CONFIG - _GLOBAL_CONFIG = DascoreConfig() - return _GLOBAL_CONFIG + with _GLOBAL_CONFIG_LOCK: + _GLOBAL_CONFIG = defaults = DascoreConfig() + return defaults diff --git a/dascore/utils/misc.py b/dascore/utils/misc.py index 1ae72c99f..62eb609f8 100644 --- a/dascore/utils/misc.py +++ b/dascore/utils/misc.py @@ -663,7 +663,7 @@ def wrapper(self, *args, **kwargs): class _MapFuncWrapper: """A class for unwrapping spools to base applies.""" - def __init__(self, func, kwargs, progress=True, config=None): + def __init__(self, func, kwargs, config, progress=True): self._func = func self._kwargs = kwargs self._progress = progress @@ -721,7 +721,7 @@ def _spool_map(spool, func, size=None, client=None, progress=True, **kwargs): # displayed in one thread/process. for sub_spool in spools[1:]: sub_spool._no_progress = True - new_func = _MapFuncWrapper(func, kwargs, progress=progress, config=get_config()) + new_func = _MapFuncWrapper(func, kwargs, get_config(), progress=progress) return [x for y in client.map(new_func, spools) for x in y] From 805ec70e70a1bd680cdc16f9522d643b54bda799 Mon Sep 17 00:00:00 2001 From: Derrick Chambers Date: Sat, 25 Jul 2026 12:06:40 +0200 Subject: [PATCH 3/3] Address PR review: forbid unknown config fields, fork-safe lock - Set extra="forbid" on DascoreConfig so misspelled overrides raise instead of being silently dropped. - Reinstall the config lock after a fork so a lock held by another thread at fork time cannot deadlock config changes in the child. - Annotate config_context's generator return type. - Show the full with-statement (and the permanent alternative) in the legacy DASDAE unpickle error message. - Document the two tiers in the configuration tutorial. - Expose the conftest permanent-config helper as a fixture so the remote common-IO module fixture stops hand-rolling save/restore, and have reset_config delegate to set_config. --- dascore/config.py | 29 +++++++++++++++++++++----- dascore/io/dasdae/_compat.py | 6 ++++-- dascore/utils/misc.py | 5 +---- docs/changelog.qmd | 2 +- docs/tutorial/configuration.qmd | 20 ++++++++++++++++++ tests/conftest.py | 6 ++++++ tests/test_io/test_remote_common_io.py | 18 ++++------------ tests/test_utils/test_config.py | 15 +++++++++++++ 8 files changed, 75 insertions(+), 26 deletions(-) diff --git a/dascore/config.py b/dascore/config.py index 262199ad8..de627f29f 100644 --- a/dascore/config.py +++ b/dascore/config.py @@ -2,6 +2,8 @@ from __future__ import annotations +import os +from collections.abc import Iterator from contextlib import contextmanager from contextvars import ContextVar from pathlib import Path @@ -29,6 +31,9 @@ class DascoreConfig(BaseModel): model_config = ConfigDict( frozen=True, validate_default=True, + # Reject unknown fields so misspelled overrides raise rather than + # silently doing nothing. + extra="forbid", ) # General behavior. @@ -152,6 +157,21 @@ def _coerce_path(cls, value): ) +def _reinit_config_lock(): + """Install a fresh config lock, used after a fork. + + A fork can copy the lock while another thread holds it. That thread does + not exist in the child, so the inherited copy would never be released and + any config change in the child would hang. + """ + global _GLOBAL_CONFIG_LOCK + _GLOBAL_CONFIG_LOCK = Lock() + + +if hasattr(os, "register_at_fork"): # not available on windows + os.register_at_fork(after_in_child=_reinit_config_lock) + + class _ConfigDescriptor: """Descriptor for attributes that should always reflect runtime config.""" @@ -228,7 +248,9 @@ def set_config(new_config: DascoreConfig | None = None, **kwargs) -> DascoreConf @contextmanager -def config_context(new_config: DascoreConfig | None = None, **kwargs): +def config_context( + new_config: DascoreConfig | None = None, **kwargs +) -> Iterator[DascoreConfig]: """ Temporarily override the runtime config for the current thread/task. @@ -269,7 +291,4 @@ def config_context(new_config: DascoreConfig | None = None, **kwargs): def reset_config() -> DascoreConfig: """Reset the process-wide runtime config base to defaults.""" - global _GLOBAL_CONFIG - with _GLOBAL_CONFIG_LOCK: - _GLOBAL_CONFIG = defaults = DascoreConfig() - return defaults + return set_config(DascoreConfig()) diff --git a/dascore/io/dasdae/_compat.py b/dascore/io/dasdae/_compat.py index c24612a0c..8db855397 100644 --- a/dascore/io/dasdae/_compat.py +++ b/dascore/io/dasdae/_compat.py @@ -63,8 +63,10 @@ def translate_legacy_attrs(attrs): msg = ( "This DASDAE file contains legacy pickled coordinate metadata. " "Unpickling DASDAE format metadata is disabled by default for " - "security. If you trust this file, enable legacy compatibility " - "with dc.config_context(allow_dasdae_format_unpickle=True)." + "security. If you trust this file, read it inside a " + "'with dc.config_context(allow_dasdae_format_unpickle=True):' " + "block, or enable it permanently with " + "dc.set_config(allow_dasdae_format_unpickle=True)." ) raise InvalidFiberFileError(msg) with contextlib.suppress( diff --git a/dascore/utils/misc.py b/dascore/utils/misc.py index 62eb609f8..306f007a6 100644 --- a/dascore/utils/misc.py +++ b/dascore/utils/misc.py @@ -23,6 +23,7 @@ from scipy.special import factorial from dascore.compat import UPath, is_array +from dascore.config import config_context, get_config from dascore.constants import WARN_LEVELS from dascore.exceptions import ( FilterValueError, @@ -673,8 +674,6 @@ def __init__(self, func, kwargs, config, progress=True): self._config = config def __call__(self, spool): - from dascore.config import config_context - with config_context(self._config): iterable = spool # in order to handle multiprocessing, we apply a secret tag of @@ -704,8 +703,6 @@ def _spool_map(spool, func, size=None, client=None, progress=True, **kwargs): **kwargs Keywords passed to func. """ - from dascore.config import get_config - # no client; simple for loop. desc = f"Applying {func.__name__} to spool" if client is None: diff --git a/docs/changelog.qmd b/docs/changelog.qmd index 641dadf17..876640642 100644 --- a/docs/changelog.qmd +++ b/docs/changelog.qmd @@ -4,7 +4,7 @@ The [releases page](https://github.com/DASDAE/dascore/releases) tracks changes f ## Unreleased API Changes -- **Runtime configuration is now two-tier.** `dc.set_config(...)` sets the process-wide base permanently (visible from every thread and task) and returns the new config; it is no longer a context manager. Temporary, thread/task-local overrides use the new `dc.config_context(...)` context manager, which restores on exit and isolates concurrent overrides via a `ContextVar`. `Spool.map(...)` captures the config active at the call and re-applies it in each worker, so thread- and process-pool workers observe the caller's config (previously process workers saw defaults). Migrate `with dc.set_config(...)` to `with dc.config_context(...)`. +- **Runtime configuration is now two-tier.** `dc.set_config(...)` sets the process-wide base permanently (visible from every thread and task) and returns the new config; it is no longer a context manager. Temporary, thread/task-local overrides use the new `dc.config_context(...)` context manager, which restores on exit and isolates concurrent overrides via a `ContextVar`. `Spool.map(...)` captures the config active at the call and re-applies it in each worker, so thread- and process-pool workers observe the caller's config regardless of the pool's start method. Unknown config field names now raise instead of being silently ignored. Migrate `with dc.set_config(...)` to `with dc.config_context(...)`. - PRODML 2.1 now supports writing one raw time-by-distance patch with `dc.write` as a standalone HDF5 file. Full PRODML 2.3 conformance requires EPC/XML packaging, which DASCore does not write. - **The `dascore.io.sintela_binary` module is removed (no alias).** Both Sintela readers now live in `dascore.io.sintela`, which also provides the new protobuf reader; use `from dascore.io.sintela import SintelaBinaryV3`. Reading Sintela binary files through `dc.read`/`dc.spool`/`dc.scan` is unaffected — only the direct module import path changed. - **Removed the unused `PatchFileSummary` model** (`dascore.io.PatchFileSummary`), superseded by [`PatchSummary`](`dascore.PatchSummary`), along with the internal helpers `coord_summary_from_data` and `_normalize_coord_summary_dtype`. Build a coord first (`get_coord(...)`) and call `.to_summary()` to summarize raw array data. The unused `index_query_buffer` config option is also removed. diff --git a/docs/tutorial/configuration.qmd b/docs/tutorial/configuration.qmd index e9220adbf..7f2f01f71 100644 --- a/docs/tutorial/configuration.qmd +++ b/docs/tutorial/configuration.qmd @@ -15,6 +15,26 @@ with config_context(remote_cache_dir=Path("/tmp/dascore-remote-cache")): Configuration changes affect subsequent operations only. For example, changing `remote_cache_dir` changes where future remote-file materializations are cached. +## Two configuration tiers + +Config can be changed in two ways: + +- `set_config(...)` changes the process-wide base permanently. The change is visible from every thread and task and is not restored automatically; `reset_config()` returns to defaults. Use it for application-level settings applied once at startup. +- `config_context(...)` overrides the config only for the current thread or task. The override is restored when the block exits, and concurrent blocks in different threads never clobber one another. + +```python +import dascore as dc + +dc.set_config(display_float_precision=5) # permanent + +with dc.config_context(display_float_precision=8): # scoped to this block + ... + +dc.reset_config() # drop the permanent change +``` + +[`Spool.map`](`dascore.core.spool.BaseSpool.map`) binds the config active when `map` is called and re-applies it in each worker, so overrides also reach thread- and process-pool workers. + History recording is also configurable: - `patch_history="standard"` preserves the default behavior and appends new entries to `Patch.attrs.history`. diff --git a/tests/conftest.py b/tests/conftest.py index 299a1580f..685168222 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -103,6 +103,12 @@ def _permanent_config(**overrides): set_config(previous) +@pytest.fixture(scope="session") +def permanent_config(): + """Return the context manager broad-scoped fixtures use to set config.""" + return _permanent_config + + @pytest.fixture(autouse=True) def use_test_config(): """Run tests with debug mode enabled unless overridden locally.""" diff --git a/tests/test_io/test_remote_common_io.py b/tests/test_io/test_remote_common_io.py index 7ec863b26..a1ded3c4c 100644 --- a/tests/test_io/test_remote_common_io.py +++ b/tests/test_io/test_remote_common_io.py @@ -7,7 +7,6 @@ import pytest import dascore as dc -from dascore.config import get_config, set_config from dascore.utils.misc import suppress_warnings from tests.test_io._common_io_test_utils import ( get_flat_io_test, @@ -61,22 +60,13 @@ def suppress_expected_remote_cache_warnings(): @pytest.fixture(scope="module", autouse=True) -def isolated_remote_cache(tmp_path_factory): - """Keep the common remote matrix in its own cache root. - - Uses the permanent config base (not a scoped ``config_context``) because a - module-scoped fixture spans many tests; the scoped override belongs to a - single call block, not a fixture that stays open across the module. - """ - previous = get_config() - set_config( +def isolated_remote_cache(tmp_path_factory, permanent_config): + """Keep the common remote matrix in its own cache root.""" + with permanent_config( remote_cache_dir=tmp_path_factory.mktemp("remote_common_cache"), allow_remote_cache_for_metadata=True, - ) - try: + ): yield - finally: - set_config(previous) def _get_remote_case(fetch_name: str, to_http_range_path): diff --git a/tests/test_utils/test_config.py b/tests/test_utils/test_config.py index 7a4a3f3c5..b924576e9 100644 --- a/tests/test_utils/test_config.py +++ b/tests/test_utils/test_config.py @@ -7,6 +7,7 @@ import pytest import dascore as dc +from dascore import config as dascore_config from dascore.config import ( DascoreConfig, config_attr, @@ -72,6 +73,20 @@ def test_sampling_group_tolerance_must_be_positive(self): with pytest.raises(ValueError, match="sampling_group_tolerance"): set_config(sampling_group_tolerance=0) + def test_unknown_field_raises(self): + """A misspelled override raises rather than being silently ignored.""" + with pytest.raises(ValueError, match="dispplay_float_precision"): + set_config(dispplay_float_precision=5) + + def test_fork_handler_replaces_held_lock(self): + """A lock held at fork time is replaced so the child cannot deadlock.""" + old_lock = dascore_config._GLOBAL_CONFIG_LOCK + with old_lock: + dascore_config._reinit_config_lock() + new_lock = dascore_config._GLOBAL_CONFIG_LOCK + assert new_lock is not old_lock + assert not new_lock.locked() + def test_visible_from_new_thread(self): """A permanent set is visible from threads launched afterward.""" set_config(display_float_precision=9)