From 17f9cdc73bd07678b3758593f3d174fe77ac601d Mon Sep 17 00:00:00 2001 From: thodson-usgs Date: Tue, 1 Sep 2026 10:52:43 -0500 Subject: [PATCH 1/5] refactor: one word per concept in the configuration chain `_resolve` returned `(raw, source, tier)`: `source` held the exact origin (`$API_USGS_RETRIES`, a path to the config file) and `tier` held the ordered category CONTEXT.md calls the source. One word, two grains, in one signature. It now returns `(raw, label, source)`. The category takes the glossary's name, and the origin string takes `label` -- the name `_show_adapter_overrides` already used for it, and the one `_parse_int` documented as "Human-readable origin". Renamed to match: `_source_label` to `_origin_label`, `_env_source_label` to `_env_label`, and `BaseConfiguration._source` to `._label`; all three build an origin label, none names a source. The rename is mechanical: applied to the parent's AST with docstrings stripped, it reproduces this commit's AST exactly, for both modules. `show_configuration()`'s output stays byte-identical. The `show_configuration` docstring and the user guide said "source" for what is an origin label; both now say origin. CONTEXT.md drops the paragraph recording that the code spelled the pair backwards. Left for the pull request: the package still says "service" in places where the glossary's term is "adapter", including the printed `""`. Each site needs its own judgement, and that one changes user-visible output. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01JAEQqs7XzQHGQQi2KakuXD --- CONTEXT.md | 5 +- dataretrieval/_configuration_core.py | 100 ++++++++++++------------ dataretrieval/configuration.py | 78 +++++++++--------- docs/source/userguide/configuration.rst | 2 +- 4 files changed, 92 insertions(+), 93 deletions(-) diff --git a/CONTEXT.md b/CONTEXT.md index eda9288d..07304687 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -171,9 +171,8 @@ selected. What `show_configuration()` prints beside each value, and what a parser names when it rejects one. A source is the category; an origin label is the instance within it. -The code carries both, and spells them the other way around: `_resolve` returns -its origin label under the name `source` and its source under the name `tier`. -Prose uses the terms above. +The code uses both names: `_resolve` returns `(raw, label, source)`, and the +parsers take the `label` as the subject of any error message they raise. **Selection** — Naming which profile an adapter should use. Done in code; a profile is never selected by the environment or implied by the file, so the diff --git a/dataretrieval/_configuration_core.py b/dataretrieval/_configuration_core.py index ee39ceee..f11cfa6f 100644 --- a/dataretrieval/_configuration_core.py +++ b/dataretrieval/_configuration_core.py @@ -66,7 +66,7 @@ #: Environment variable holding an explicit path to the configuration file. CONFIG_PATH_ENV = "DATARETRIEVAL_CONFIG" -#: Source label for a setting no source supplied. +#: Origin label for a setting no source supplied. _BUILT_IN = "built-in default" #: The table ADR 0011 retired. Named here only so a file written against the @@ -117,7 +117,7 @@ # is absent on a fork), so treating it as configured would let it shadow the # config file and silently drop the user's API key. Keeping this a property of # the setting -- rather than a second, lower visit to the environment -- keeps -# the chain at the three tiers the docstring and ADR 0009 describe. +# the chain at the three sources the docstring and ADR 0009 describe. _BLANK_MEANS_SET = frozenset({"progress"}) # Warnings about the config file report the file, not a call site: settings are @@ -157,7 +157,7 @@ def __repr__(self) -> str: # inner package-wide one, inverting the nesting rule ADR 0011 states. # # Each entry pairs the raw value with the label naming where it came from, the -# same shape the file tier returns (:func:`_adapter_file_settings`). The +# same shape the file source returns (:func:`_adapter_file_settings`). The # label is built while the configuration object is still in hand, the only # point where the *profile* is known -- by the time a value reaches the frame, # one from ``WaterdataConfiguration.load("bulk")`` and one from @@ -295,7 +295,7 @@ def __post_init__(self) -> None: if value is not None: # ``None`` is not a value to check: it means "suppress the # lower sources", which every setting accepts. - _validated_raw(name, value, self._source(name), optional=", or None") + _validated_raw(name, value, self._label(name), optional=", or None") self.validate() def validate(self) -> None: @@ -332,7 +332,7 @@ def load(cls: type[_C], profile: str) -> _C: ``[.]``. Only the keys that table names are carried, so the profile still inherits the adapter's default profile and the - package-wide keys per setting from the tiers below. + package-wide keys per setting from the sources below. Selecting a profile the file does not define raises: a name a caller just typed is a typo worth reporting, not a silent fall-through to @@ -364,7 +364,7 @@ def load(cls: type[_C], profile: str) -> _C: object.__setattr__(loaded, "profile", profile) return loaded - def _source(self, name: str) -> str: + def _label(self, name: str) -> str: """How one of this configuration's settings is named in an error.""" return f"{name}= in {type(self).__name__}()" @@ -563,8 +563,8 @@ def settings_for(adapter: str) -> frozenset[str] | None: return None if cls is None else cls.settings() -def _env_source_label(env_var: str) -> str: - """How a value read from ``env_var`` is reported as a source.""" +def _env_label(env_var: str) -> str: + """How a value read from ``env_var`` is reported as an origin label.""" return f"${env_var}" @@ -720,46 +720,46 @@ def _home_id() -> str: # TOML types and reject Python API type errors before producing raw strings. -def _type_error(source: str, expected: str, value: object) -> ConfigurationError: +def _type_error(label: str, expected: str, value: object) -> ConfigurationError: """Build a type error without rendering a possibly secret value.""" return ConfigurationError( - f"{source} must be {expected} (got {type(value).__name__})." + f"{label} must be {expected} (got {type(value).__name__})." ) -def _coerce_string(value: object, source: str, optional: str) -> str: +def _coerce_string(value: object, label: str, optional: str) -> str: if not isinstance(value, str): - raise _type_error(source, "a string" + optional, value) + raise _type_error(label, "a string" + optional, value) return value -def _coerce_progress(value: object, source: str, optional: str) -> str: +def _coerce_progress(value: object, label: str, optional: str) -> str: if isinstance(value, bool): return str(value) if isinstance(value, str): return value - raise _type_error(source, "a bool or recognized string" + optional, value) + raise _type_error(label, "a bool or recognized string" + optional, value) -def _coerce_concurrency(value: object, source: str, optional: str) -> str: +def _coerce_concurrency(value: object, label: str, optional: str) -> str: if isinstance(value, bool) or not isinstance(value, (Integral, str)): - raise _type_error(source, "an integer or 'unbounded'" + optional, value) + raise _type_error(label, "an integer or 'unbounded'" + optional, value) if isinstance(value, str) and value.strip().lower() != CONCURRENCY_UNBOUNDED: - raise ConfigurationError(f"{source} must be an integer or 'unbounded'.") + raise ConfigurationError(f"{label} must be an integer or 'unbounded'.") return str(value) -def _coerce_seconds(value: object, source: str, optional: str) -> str: +def _coerce_seconds(value: object, label: str, optional: str) -> str: # Seconds, so a fractional value is meaningful -- unlike the counts, which # are whole by nature. if isinstance(value, bool) or not isinstance(value, (Integral, float)): - raise _type_error(source, "a number of seconds" + optional, value) + raise _type_error(label, "a number of seconds" + optional, value) return str(value) -def _coerce_count(value: object, source: str, optional: str) -> str: +def _coerce_count(value: object, label: str, optional: str) -> str: if isinstance(value, bool) or not isinstance(value, Integral): - raise _type_error(source, "an integer" + optional, value) + raise _type_error(label, "an integer" + optional, value) return str(value) @@ -791,7 +791,7 @@ def _coerce_count(value: object, source: str, optional: str) -> str: ) -def _coerce_typed(name: str, value: object, source: str, *, optional: str = "") -> str: +def _coerce_typed(name: str, value: object, label: str, *, optional: str = "") -> str: """Type-check one source-level value and render it as a raw string. Shared by the two *typed* surfaces -- a configuration's fields and TOML @@ -802,19 +802,19 @@ def _coerce_typed(name: str, value: object, source: str, *, optional: str = "") ``optional`` is the only thing that differs between them: the Python surface accepts ``None`` and says so in its messages. """ - return _TYPES[name](value, source, optional) + return _TYPES[name](value, label, optional) -def _validated_raw(name: str, value: object, source: str, *, optional: str = "") -> str: +def _validated_raw(name: str, value: object, label: str, *, optional: str = "") -> str: """Type-check, render and grammar-check one typed value.""" - raw = _coerce_typed(name, value, source, optional=optional) - _validate_raw(name, raw, source) + raw = _coerce_typed(name, value, label, optional=optional) + _validate_raw(name, raw, label) return raw def _parse_int( raw: str, - source: str, + label: str, *, default: int, minimum: int, @@ -826,8 +826,8 @@ def _parse_int( ---------- raw : str The value as written, from whichever source supplied it. - source : str - Human-readable origin, used as the subject of any error message. + label : str + Human-readable origin label, used as the subject of any error message. default : int Returned for a blank value, matching the environment-variable behavior this replaced. @@ -843,13 +843,13 @@ def _parse_int( try: parsed = int(value) except ValueError as exc: - raise ConfigurationError(f"{source} must be {expected} (got {raw!r}).") from exc + raise ConfigurationError(f"{label} must be {expected} (got {raw!r}).") from exc if parsed < minimum: - raise ConfigurationError(f"{source} must be {expected} (got {parsed}).") + raise ConfigurationError(f"{label} must be {expected} (got {parsed}).") return parsed -def _parse_seconds(raw: str, source: str) -> float: +def _parse_seconds(raw: str, label: str) -> float: """Parse a non-negative duration in seconds; blank falls through. Seconds rather than a count, so fractional values are accepted. ``0`` @@ -863,29 +863,29 @@ def _parse_seconds(raw: str, source: str) -> float: try: parsed = float(value) except ValueError as exc: - raise ConfigurationError(f"{source} must be {expected} (got {raw!r}).") from exc + raise ConfigurationError(f"{label} must be {expected} (got {raw!r}).") from exc # ``inf`` and ``nan`` both parse as floats and both defeat the bound they # are meant to set: ``inf`` makes every wait allowed, and ``nan`` compares # false against every threshold. TOML has literal ``inf``/``nan``, so this # is reachable from the file as well as from Python. if not math.isfinite(parsed) or parsed < 0: - raise ConfigurationError(f"{source} must be {expected} (got {parsed}).") + raise ConfigurationError(f"{label} must be {expected} (got {parsed}).") return parsed -def _parse_concurrency(raw: str, source: str) -> int | None: +def _parse_concurrency(raw: str, label: str) -> int | None: """Parse a concurrency cap: a positive int, or ``unbounded`` -> ``None``.""" if raw.strip().lower() == CONCURRENCY_UNBOUNDED: return None try: - return _parse_int(raw, source, default=DEFAULT_CONCURRENCY, minimum=1) + return _parse_int(raw, label, default=DEFAULT_CONCURRENCY, minimum=1) except ConfigurationError as exc: raise ConfigurationError( f"{exc} Use '{CONCURRENCY_UNBOUNDED}' to disable the cap." ) from exc -def _parse_base_url(raw: str, source: str) -> str: +def _parse_base_url(raw: str, label: str) -> str: """Parse a service base URL: an absolute ``http``/``https`` origin. Only the scheme is checked, and deliberately so. This module cannot know @@ -896,16 +896,16 @@ def _parse_base_url(raw: str, source: str) -> str: value = raw.strip() if not value.startswith(("http://", "https://")): raise ConfigurationError( - f"{source} must be an absolute http:// or https:// URL (got {raw!r})." + f"{label} must be an absolute http:// or https:// URL (got {raw!r})." ) return value -def _parse_progress(raw: str, source: str, *, strict: bool) -> bool: +def _parse_progress(raw: str, label: str, *, strict: bool) -> bool: """Parse a progress toggle, optionally preserving legacy env truthiness.""" value = raw.strip().lower() if strict and not value: - raise ConfigurationError(f"{source} must not be blank.") + raise ConfigurationError(f"{label} must not be blank.") if value in _PROGRESS_FALSEY: return False if value in _PROGRESS_TRUTHY: @@ -913,7 +913,7 @@ def _parse_progress(raw: str, source: str, *, strict: bool) -> bool: if not strict: return True expected = ", ".join(sorted(_PROGRESS_TRUTHY | _PROGRESS_FALSEY)) - raise ConfigurationError(f"{source} must be one of {expected} (got {raw!r}).") + raise ConfigurationError(f"{label} must be one of {expected} (got {raw!r}).") # Each integer setting's grammar, named once. The accessor and the eager @@ -936,11 +936,11 @@ def _parse_progress(raw: str, source: str, *, strict: bool) -> bool: } -def _validate_raw(name: str, raw: str, source: str) -> None: +def _validate_raw(name: str, raw: str, label: str) -> None: """Run a setting's grammar validator when it has one.""" validate = _VALIDATORS.get(name) if validate is not None: - validate(raw, source) + validate(raw, label) def _named_profiles(parsed: _ParsedFile, adapter: str) -> dict[str, dict[str, Any]]: @@ -972,7 +972,7 @@ def _named_profile( Returns the TOML scalars as written rather than raw strings, because the caller is :meth:`BaseConfiguration.load`, which feeds them straight back into the configuration's own typed fields. Values are still checked here, - with a source that names the file and the table: a grammar error found on + with a label that names the file and the table: a grammar error found on the way *out* of the file should say which line to fix, not merely which field of which class ended up holding it. """ @@ -1019,8 +1019,8 @@ def _current_file() -> tuple[Path, _ParsedFile]: """The config file as currently loaded: its path and its parsed form. One helper so the two always travel together. They are a single fact, and - handing the top-level tier a different ``_ParsedFile`` than the adapter - tier saw in the same resolution is exactly the drift that made an + handing the top-level source a different ``_ParsedFile`` than the adapter + source saw in the same resolution is exactly the drift that made an adapter-scoped read load the file twice. """ path = config_path() @@ -1033,7 +1033,7 @@ def _adapter_file_settings( """The ``[]`` table's own keys -- its default profile. Layers *above* the file's top-level keys rather than being merged into - them: within the file tier an adapter's own value outranks the package-wide + them: within the file source an adapter's own value outranks the package-wide one. The table's sub-tables are its named profiles, which are inert until a caller selects one, so they are skipped here (see :func:`_accepted_keys`). @@ -1299,9 +1299,9 @@ def _checked_table( UserWarning, stacklevel=_WARN_STACKLEVEL, ) - source = f"{path}: {key!r} at {where}" - raw = _coerce_typed(key, value, source) - _validate_raw(key, raw, source) + label = f"{path}: {key!r} at {where}" + raw = _coerce_typed(key, value, label) + _validate_raw(key, raw, label) checked[key] = (value, raw) return checked diff --git a/dataretrieval/configuration.py b/dataretrieval/configuration.py index d856ee9e..e7408653 100644 --- a/dataretrieval/configuration.py +++ b/dataretrieval/configuration.py @@ -90,7 +90,7 @@ _coerce_typed as _coerce_typed, _Concurrent as _Concurrent, _current_file as _current_file, - _env_source_label as _env_source_label, + _env_label as _env_label, _Frame as _Frame, _named_profiles as _named_profiles, _NO_FILE as _NO_FILE, @@ -260,17 +260,17 @@ def _configuration_overrides( raw = ( None if value is None - else _coerce_typed(name, value, configuration._source(name)) + else _coerce_typed(name, value, configuration._label(name)) ) overrides[key] = (raw, label) return overrides def show_configuration(*, stream: TextIO | None = None) -> None: - """Print the effective configuration and the source of each setting. + """Print the effective configuration and where each setting came from. A debugging aid for "why is this using my old key?". Every value is - reported with the source that supplied it, named exactly: which variable, + reported with the origin that supplied it, named exactly: which variable, which table of the file, and -- when a caller selected one -- which profile. The API key is never printed, only whether one is set. @@ -325,12 +325,12 @@ def show_configuration(*, stream: TextIO | None = None) -> None: parsed = _show_file_status(out, path, cell) rows = [ - (name, cell(partial(_DISPLAYS[name], None)), cell(partial(_source_label, name))) + (name, cell(partial(_DISPLAYS[name], None)), cell(partial(_origin_label, name))) for name in SETTINGS ] _print_setting_rows(out, rows) _show_built_in_default_note(out, rows) - _show_adapter_overrides(out, cell, {name: source for name, _value, source in rows}) + _show_adapter_overrides(out, cell, {name: label for name, _value, label in rows}) _show_profiles(out, parsed) _show_unimported_adapters(out) @@ -386,15 +386,15 @@ def _show_file_status( def _print_setting_rows(out: TextIO, rows: list[tuple[str, str, str]]) -> None: """Print the package-wide setting rows in aligned columns.""" - name_width = max(len(name) for name, _value, _source in rows) - value_width = max(len(value) for _name, value, _source in rows) - for name, value, source in rows: - print(f"{name:<{name_width}} {value:<{value_width}} {source}", file=out) + name_width = max(len(name) for name, _value, _label in rows) + value_width = max(len(value) for _name, value, _label in rows) + for name, value, label in rows: + print(f"{name:<{name_width}} {value:<{value_width}} {label}", file=out) def _show_built_in_default_note(out: TextIO, rows: list[tuple[str, str, str]]) -> None: """Print the built-in default footnote when at least one row uses it.""" - if any(source == _BUILT_IN for _name, _value, source in rows): + if any(label == _BUILT_IN for _name, _value, label in rows): print( "\nA built-in default is package-wide. An adapter may prefer its own " "for\nits own calls; a value from any source above overrides both.", @@ -413,7 +413,7 @@ def _show_adapter_overrides( full adapter-by-setting grid would be mostly inherited values, burying the answer to "what will this call use" under the rows that change nothing. - Each row names its source exactly, which for a selected profile is the + Each row names its origin exactly, which for a selected profile is the profile: ``configure() block [waterdata.bulk]`` rather than a bare block, so the report answers *which* profile put that value there. @@ -428,9 +428,9 @@ def _show_adapter_overrides( a_width = max(len(a) for a, _n, _v, _s in overrides) n_width = max(len(n) for _a, n, _v, _s in overrides) v_width = max(len(v) for _a, _n, v, _s in overrides) - for adapter, name, value, source in overrides: + for adapter, name, value, label in overrides: print( - f" {adapter:<{a_width}} {name:<{n_width}} {value:<{v_width}} {source}", + f" {adapter:<{a_width}} {name:<{n_width}} {value:<{v_width}} {label}", file=out, ) @@ -469,9 +469,9 @@ def _overrides_for_adapter( for name in _ALL_SETTINGS: if name not in accepted: continue - scoped = cell(partial(_source_label, name, adapter)) + scoped = cell(partial(_origin_label, name, adapter)) if scoped == package_wide.get(name, _BUILT_IN): - continue # inherited from the package-wide tier + continue # inherited from the package-wide source value = cell(partial(_DISPLAYS[name], adapter)) overrides.append((adapter, name, value, scoped)) return overrides @@ -525,8 +525,8 @@ def _show_unimported_adapters(out: TextIO) -> None: ) -def _source_label(name: str, adapter: str | None = None) -> str: - """The provenance label for one setting, for :func:`show_configuration`.""" +def _origin_label(name: str, adapter: str | None = None) -> str: + """The origin label for one setting, for :func:`show_configuration`.""" return _resolve(name, adapter)[1] @@ -539,7 +539,7 @@ def api_key() -> str | None: Surrounding whitespace is stripped, so a key read from a file with a trailing newline works; a blank value resolves to ``None``. """ - raw, _source, _tier = _resolve("api_key") + raw, _label, _source = _resolve("api_key") return raw.strip() or None if raw is not None else None @@ -554,18 +554,18 @@ def concurrency( wins over it: a service able to override an explicit setting would make ``concurrency=1`` a lie. """ - raw, source, _tier = _resolve("concurrency", adapter) + raw, label, _source = _resolve("concurrency", adapter) if raw is None: return default - return _parse_concurrency(raw, source) + return _parse_concurrency(raw, label) def retries(*, adapter: str | None = None) -> int: """Retries attempted after the first try; ``0`` disables retrying.""" - raw, source, _tier = _resolve("retries", adapter) + raw, label, _source = _resolve("retries", adapter) if raw is None: return DEFAULT_RETRIES - return _parse_retries(raw, source) + return _parse_retries(raw, label) def progress() -> bool | None: @@ -575,12 +575,12 @@ def progress() -> bool | None: default (a TTY or Jupyter kernel gets the line, redirected output doesn't). """ - raw, source, tier = _resolve("progress") + raw, label, source = _resolve("progress") if raw is None: return None # Preserve the legacy environment behavior (any value outside the false # set enables progress), while new block/file values are validated strictly. - return _parse_progress(raw, source, strict=tier != _ENV) + return _parse_progress(raw, label, strict=source != _ENV) def parallel_chunks(*, adapter: str | None = None) -> int: @@ -592,10 +592,10 @@ def parallel_chunks(*, adapter: str | None = None) -> int: the name of that context manager because it is the same setting -- this is the resolved value, not the scoping block. """ - raw, source, _tier = _resolve("parallel_chunks", adapter) + raw, label, _source = _resolve("parallel_chunks", adapter) if raw is None: return DEFAULT_PARALLEL_CHUNKS - return _parse_parallel_chunks(raw, source) + return _parse_parallel_chunks(raw, label) def stall_timeout(*, adapter: str | None = None) -> float: @@ -605,10 +605,10 @@ def stall_timeout(*, adapter: str | None = None) -> float: connection, which the retry *count* does not: it counts attempts, not seconds. See :attr:`dataretrieval.transport.retry.RetryPolicy.stall_timeout`. """ - raw, source, _tier = _resolve("stall_timeout", adapter) + raw, label, _source = _resolve("stall_timeout", adapter) if raw is None: return DEFAULT_STALL_TIMEOUT - return _parse_seconds(raw, source) + return _parse_seconds(raw, label) @overload @@ -647,23 +647,23 @@ def base_url(*, adapter: str | None = None, default: str | None = None) -> str | the answer is ``None`` -- which is what :func:`show_configuration` asks for, having no service default to name. """ - raw, source, _tier = _resolve("base_url", adapter) + raw, label, _source = _resolve("base_url", adapter) if raw is None: return default - return _parse_base_url(raw, source) + return _parse_base_url(raw, label) # --- resolution ---------------------------------------------------------- -#: Which tier of the chain answered a resolution. Machine-readable so a -#: per-tier rule reads the tier, never the display label -- :func:`progress` +#: Which source of the chain answered a resolution. Machine-readable so a +#: per-source rule reads the source, never the display label -- :func:`progress` #: keys its legacy-lenient parsing on ``_ENV``, and the label stays purely #: presentational. _BLOCK, _ENV, _FILE, _DEFAULT = "block", "environment", "file", "built-in" def _resolve(name: str, adapter: str | None = None) -> tuple[str | None, str, str]: - """Return the raw value for *name*, a source label, and the tier. + """Return the raw value for *name*, a label label, and the tier. Precedence is *source-major*: the chain walks block, then environment, then file, exactly as ADR 0009 defines it -- and *within* each source an @@ -681,8 +681,8 @@ def _resolve(name: str, adapter: str | None = None) -> tuple[str | None, str, st ------- tuple[str or None, str, str] The raw string as written (parsing happens per setting, so each keeps - its own blank-value rule), the human-readable source label, and which - tier answered (one of the constants above) -- ``None`` with + its own blank-value rule), the human-readable label label, and which + source answered (one of the constants above) -- ``None`` with ``_BUILT_IN`` / ``_DEFAULT`` when nothing configured it. """ _check_adapter_known(adapter) @@ -733,7 +733,7 @@ def _check_env_not_refused(name: str) -> None: refused = _REFUSED_ENV_VARS.get(name) if refused is not None and refused in os.environ: raise ConfigurationError( - f"{_env_source_label(refused)} is set, but {name!r} may only be set " + f"{_env_label(refused)} is set, but {name!r} may only be set " "in code, in a configure() block, never from the environment. Unset " f"it and pass the value on the adapter's configuration, e.g. " f"WaterdataConfiguration({name}=...)." @@ -769,14 +769,14 @@ def _resolve_from_env(name: str) -> tuple[str | None, str, str] | None: return None raw = os.environ.get(env) if raw is not None and (raw.strip() or name in _BLANK_MEANS_SET): - return raw, _env_source_label(env), _ENV + return raw, _env_label(env), _ENV return None def _resolve_from_file(name: str, scoped: str | None) -> tuple[str | None, str, str]: """Fall through to the configuration file, then the built-in default. - One load serves both file tiers. + One load serves both file sources. """ path, parsed = _current_file() diff --git a/docs/source/userguide/configuration.rst b/docs/source/userguide/configuration.rst index 85472629..223a0dd3 100644 --- a/docs/source/userguide/configuration.rst +++ b/docs/source/userguide/configuration.rst @@ -437,7 +437,7 @@ the ``bulk`` profile selected for the block: not reported: nldi (not imported, so the settings each accepts are unknown here) -Each line names the exact source, including which table inside the file, which +Each line names the exact origin, including which table inside the file, which is usually enough to answer "why is it still using my old key?". A value that came from a profile names the profile — ``configure() block [waterdata.bulk]``, not merely "a block" — so a report taken from inside a From 934d4ed5d5db40f66215ca41069eef6285d0074b Mon Sep 17 00:00:00 2001 From: thodson-usgs Date: Tue, 1 Sep 2026 11:48:38 -0500 Subject: [PATCH 2/5] docs: fix the wording the rename got wrong The rename proved behaviour unchanged by comparing ASTs with docstrings stripped -- silent, that is, about exactly the text a rename most easily breaks. Reading the diff found three kinds of damage: Breakage: `\bsource\b` had matched inside "source label", leaving "a label label" in `_resolve`'s summary and Returns section, and the summary still said "tier". Category errors: three sentences got "source" where the glossary means scope -- the file's adapter-scoped and package-wide lookups are two scopes within one source, not two sources. Leftovers: `tier` survived in `waterdata/configuration.py` and in four test docstrings the rename never covered; `_configuration_core` counted "three sources" where its module docstring numbers four, and dropped a preposition; and `WaterdataConfiguration` said "this service" and "every other adapter" for one thing in one sentence -- the scope is the adapter. Prose only; the AST comparison against upstream/main still holds. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01JAEQqs7XzQHGQQi2KakuXD --- dataretrieval/_configuration_core.py | 6 +++--- dataretrieval/configuration.py | 8 ++++---- dataretrieval/waterdata/configuration.py | 2 +- tests/configuration_test.py | 8 ++++---- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/dataretrieval/_configuration_core.py b/dataretrieval/_configuration_core.py index f11cfa6f..b5fcbaa5 100644 --- a/dataretrieval/_configuration_core.py +++ b/dataretrieval/_configuration_core.py @@ -117,7 +117,7 @@ # is absent on a fork), so treating it as configured would let it shadow the # config file and silently drop the user's API key. Keeping this a property of # the setting -- rather than a second, lower visit to the environment -- keeps -# the chain at the three sources the docstring and ADR 0009 describe. +# the chain in the shape the docstring and ADR 0009 describe. _BLANK_MEANS_SET = frozenset({"progress"}) # Warnings about the config file report the file, not a call site: settings are @@ -1019,8 +1019,8 @@ def _current_file() -> tuple[Path, _ParsedFile]: """The config file as currently loaded: its path and its parsed form. One helper so the two always travel together. They are a single fact, and - handing the top-level source a different ``_ParsedFile`` than the adapter - source saw in the same resolution is exactly the drift that made an + handing the top-level scope a different ``_ParsedFile`` than the + adapter scope saw in the same resolution is exactly the drift that made an adapter-scoped read load the file twice. """ path = config_path() diff --git a/dataretrieval/configuration.py b/dataretrieval/configuration.py index e7408653..157fecb2 100644 --- a/dataretrieval/configuration.py +++ b/dataretrieval/configuration.py @@ -471,7 +471,7 @@ def _overrides_for_adapter( continue scoped = cell(partial(_origin_label, name, adapter)) if scoped == package_wide.get(name, _BUILT_IN): - continue # inherited from the package-wide source + continue # inherited from the package-wide value value = cell(partial(_DISPLAYS[name], adapter)) overrides.append((adapter, name, value, scoped)) return overrides @@ -663,7 +663,7 @@ def base_url(*, adapter: str | None = None, default: str | None = None) -> str | def _resolve(name: str, adapter: str | None = None) -> tuple[str | None, str, str]: - """Return the raw value for *name*, a label label, and the tier. + """Return the raw value for *name*, its origin label, and its source. Precedence is *source-major*: the chain walks block, then environment, then file, exactly as ADR 0009 defines it -- and *within* each source an @@ -681,7 +681,7 @@ def _resolve(name: str, adapter: str | None = None) -> tuple[str | None, str, st ------- tuple[str or None, str, str] The raw string as written (parsing happens per setting, so each keeps - its own blank-value rule), the human-readable label label, and which + its own blank-value rule), the human-readable origin label, and which source answered (one of the constants above) -- ``None`` with ``_BUILT_IN`` / ``_DEFAULT`` when nothing configured it. """ @@ -776,7 +776,7 @@ def _resolve_from_env(name: str) -> tuple[str | None, str, str] | None: def _resolve_from_file(name: str, scoped: str | None) -> tuple[str | None, str, str]: """Fall through to the configuration file, then the built-in default. - One load serves both file sources. + One load serves both scopes within the file. """ path, parsed = _current_file() diff --git a/dataretrieval/waterdata/configuration.py b/dataretrieval/waterdata/configuration.py index 8813b3c4..356d1032 100644 --- a/dataretrieval/waterdata/configuration.py +++ b/dataretrieval/waterdata/configuration.py @@ -31,7 +31,7 @@ class WaterdataConfiguration( """Settings for Water Data calls alone. Pass one to :func:`dataretrieval.configure` to narrow a setting to this - service, leaving every other adapter on whatever the tiers below it + adapter, leaving every other adapter on whatever the sources below it resolve:: with dataretrieval.configure(WaterdataConfiguration(concurrency=8)): diff --git a/tests/configuration_test.py b/tests/configuration_test.py index c0b6775e..dc49fbe4 100644 --- a/tests/configuration_test.py +++ b/tests/configuration_test.py @@ -232,7 +232,7 @@ def test_two_configurations_for_one_adapter_raise(): def test_a_configuration_resolves_end_to_end(config_file, monkeypatch): - """Every tier below a passed configuration still applies, per setting.""" + """Every source below a passed configuration still applies, per setting.""" config_file('api_key = "file-key"\nstall_timeout = 15\n') monkeypatch.setenv("API_USGS_RETRIES", "9") @@ -359,7 +359,7 @@ def test_several_named_profiles_are_selected_independently(config_file): def test_a_named_profile_layers_per_key_over_the_tiers_below(config_file): - """Selecting a profile replaces keys, never whole tiers. + """Selecting a profile replaces keys, never whole sources. Every level of the file overrides the one below it *per key* (ADR 0011), so one adapter-scoped read here draws each of its four settings from a @@ -1073,7 +1073,7 @@ def test_unknown_setting_in_an_unselected_profile_is_silent(config_file, recwarn def test_a_malformed_table_does_not_fail_another_adapters_call(config_file): - """The blast-radius rule, on the tier a whole adapter table sits in. + """The blast-radius rule, on the source a whole adapter table sits in. Keys are checked when *that* adapter first resolves a setting, so an invalid value in ``[nldi]`` costs a Water Data call nothing -- which is @@ -1174,7 +1174,7 @@ def test_one_block_configures_several_adapters(config_file): def test_environment_outranks_an_adapter_table(config_file, monkeypatch): - """Precedence is source-major: the env tier is above the file tier. + """Precedence is source-major: the env source is above the file source. Scope-major ordering would invert this the moment anyone added an adapter table, so a variable exported for one run would lose to a stale file entry. From b2427049ad28366affb49d9735d441849a18defb Mon Sep 17 00:00:00 2001 From: thodson-usgs Date: Tue, 1 Sep 2026 14:19:57 -0500 Subject: [PATCH 3/5] test: call the ladder positions rungs, as ADR 0011 does The glossary now defines the precedence ladder and its rungs (PR #405), which makes the one identifier still saying "tiers" for ladder positions a core-term misspelling rather than a stray. The file's own comments already say rung. ADR 0011's compliance citation is annotated the way ADR 0008 annotated the wateruse rename, so the reference stays true. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01JAEQqs7XzQHGQQi2KakuXD --- .../architecture/decisions/0011-configuration-profiles.rst | 3 ++- tests/configuration_test.py | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/source/architecture/decisions/0011-configuration-profiles.rst b/docs/source/architecture/decisions/0011-configuration-profiles.rst index db0c34b9..49181f5d 100644 --- a/docs/source/architecture/decisions/0011-configuration-profiles.rst +++ b/docs/source/architecture/decisions/0011-configuration-profiles.rst @@ -210,7 +210,8 @@ Satisfied. In ``tests/configuration_test.py``: - ``test_several_named_profiles_are_selected_independently`` -- one block, a different profile per adapter. -- ``test_a_named_profile_layers_per_key_over_the_tiers_below`` -- a profile +- ``test_a_named_profile_layers_per_key_over_the_rungs_below`` (named + ``..._the_tiers_below`` when this decision was taken) -- a profile inherits its adapter's default profile and the package-wide keys per key. - ``test_adding_a_named_profile_changes_nothing_until_it_is_selected`` -- a named profile is inert until something selects it. diff --git a/tests/configuration_test.py b/tests/configuration_test.py index dc49fbe4..476f5f82 100644 --- a/tests/configuration_test.py +++ b/tests/configuration_test.py @@ -358,7 +358,7 @@ def test_several_named_profiles_are_selected_independently(config_file): assert configuration.concurrency(adapter="ngwmn") == 4 -def test_a_named_profile_layers_per_key_over_the_tiers_below(config_file): +def test_a_named_profile_layers_per_key_over_the_rungs_below(config_file): """Selecting a profile replaces keys, never whole sources. Every level of the file overrides the one below it *per key* (ADR 0011), so From f5f29ee8e1ec1452d6dc940c88e57e0dacba0506 Mon Sep 17 00:00:00 2001 From: thodson-usgs Date: Tue, 1 Sep 2026 17:56:03 -0500 Subject: [PATCH 4/5] docs: say rung where the ladder is meant, not scope Two docstrings called a rung a scope. The module docstring said ADR 0011 splits three of the four sources "into the scopes they contain", but the block source's split -- a passed instance vs a profile loaded from the file -- is the same scope either way. It now counts the ladder plainly: seven rungs over four sources, three of which hold two apiece. And the renamed test's docstring said a profile "replaces keys, never whole sources", where what it does not replace wholesale is the rung. Both wordings predate the glossary's scope/rung split; this PR owes them one word per concept. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01JAEQqs7XzQHGQQi2KakuXD --- dataretrieval/configuration.py | 3 ++- tests/configuration_test.py | 6 +++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/dataretrieval/configuration.py b/dataretrieval/configuration.py index 157fecb2..bfde1bca 100644 --- a/dataretrieval/configuration.py +++ b/dataretrieval/configuration.py @@ -19,7 +19,8 @@ 4. The built-in default. Each of those four sources is one branch of :func:`_resolve`; ADR 0011 lists the -same order in finer grain, splitting three of them into the scopes they contain. +same order in finer grain, as seven rungs -- three of these sources hold two +apiece. An adapter's own default is not one of the four: a read site such as :func:`concurrency` passes it in as the ``default`` argument. diff --git a/tests/configuration_test.py b/tests/configuration_test.py index 476f5f82..c1d4bd28 100644 --- a/tests/configuration_test.py +++ b/tests/configuration_test.py @@ -359,10 +359,10 @@ def test_several_named_profiles_are_selected_independently(config_file): def test_a_named_profile_layers_per_key_over_the_rungs_below(config_file): - """Selecting a profile replaces keys, never whole sources. + """Selecting a profile replaces keys, never whole rungs. - Every level of the file overrides the one below it *per key* (ADR 0011), so - one adapter-scoped read here draws each of its four settings from a + Every rung overrides the one below it *per key* (ADR 0011), so one + adapter-scoped read here draws each of its four settings from a different table. """ config_file( From 590a683545df56a13ee976d98ac868886efc68d0 Mon Sep 17 00:00:00 2001 From: thodson-usgs Date: Tue, 1 Sep 2026 20:42:43 -0500 Subject: [PATCH 5/5] docs: name the rungs a selected profile inherits from `BaseConfiguration.load` said a selected profile inherits the adapter's default profile and the package-wide keys "from the sources below". Both sit inside the file source -- rungs 4 and 5 -- so the sentence names positions, which is what CONTEXT.md reserves *rung* for. The user guide has said "the rungs below" since #405; the two now agree. Also joins the "apiece." the ladder sentence left orphaned on its own line. --- dataretrieval/_configuration_core.py | 2 +- dataretrieval/configuration.py | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/dataretrieval/_configuration_core.py b/dataretrieval/_configuration_core.py index b5fcbaa5..3e623ed3 100644 --- a/dataretrieval/_configuration_core.py +++ b/dataretrieval/_configuration_core.py @@ -332,7 +332,7 @@ def load(cls: type[_C], profile: str) -> _C: ``[.]``. Only the keys that table names are carried, so the profile still inherits the adapter's default profile and the - package-wide keys per setting from the sources below. + package-wide keys per setting from the rungs below. Selecting a profile the file does not define raises: a name a caller just typed is a typo worth reporting, not a silent fall-through to diff --git a/dataretrieval/configuration.py b/dataretrieval/configuration.py index bfde1bca..0fb95130 100644 --- a/dataretrieval/configuration.py +++ b/dataretrieval/configuration.py @@ -20,8 +20,7 @@ Each of those four sources is one branch of :func:`_resolve`; ADR 0011 lists the same order in finer grain, as seven rungs -- three of these sources hold two -apiece. -An adapter's own default is not one of the four: a read site such as +apiece. An adapter's own default is not one of the four: a read site such as :func:`concurrency` passes it in as the ``default`` argument. Precedence applies **per setting**, not per source: an environment that sets only