DOI-USGS / DOI-USGS/dataretrieval-python

Decompose utils.py: Ca=19 hub drives the dependency-depth and coupling scores

Open
#361 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
265
Forks
63
Avg merge
1d 20h
Merged PRs (30d)
19

Description

The weekly `pyscn` sweep added in #360 scores the package **78/100 (B)**. Two sub-scores carry almost the whole deficit, and both trace back to one module. Filing the analysis so the work is scoped rather than rediscovered.

## `utils.py` is the hub

| metric | value |
|---|---|
| afferent coupling (Ca) | **19** — the highest in the package; next is 11 |
| lines | 528 |
| responsibilities | metadata, frame shaping, ambient config, legacy request composition, transport compatibility |

The architecture overview already lists this under *Known architectural debt*: "`utils.py` combines metadata, shaping, ambient configuration, legacy request composition, and transport compatibility imports." This is the measured version of that note.

The cost is concrete. Because `utils` imports the HTTP stack (`transport.http`, `transport.retry`, `credentials`, `exceptions`, `codes`), anything that needs *one* helper from it inherits all of that transitively. The clearest case:

`ogc/context.py` is 20 lines. Its only substantive import is `Ambient` — a `ContextVar` wrapper that depends on nothing but `contextvars` and `contextlib`. Importing it from `utils` puts the entire HTTP stack in `ogc.context`'s transitive closure, and lands it on the longest dependency chain in the package:

```
dataretrieval -> waterdata -> waterdata.nearest -> waterdata.time_series
-> waterdata.stats -> waterdata.utils -> ogc -> ogc.engine -> ogc.shaping
-> ogc.schema -> ogc.context -> utils -> transport.retry -> progress -> credentials
```

Note `Ambient` is not used by `utils` itself — only by `ogc.context` and `ogc.chunking`. It lives there by accident.

## Suggested first step

Move `Ambient` to a dependency-free leaf module (`dataretrieval/ambient.py`). Small, self-contained, two import sites.

**Measured honestly: this alone moves max graph depth 14 → 13**, because `ogc.schema -> ogc.errors -> transport.pagination` is a nearly-as-long parallel path. So do it for the coupling and the clarity, not for the score — the number barely moves until the wider decomposition happens. Adding it to `.importlinter`'s layer stack is required (the stack is `exhaustive`), which is the contract doing its job.

## The wider work

Split `utils.py` along the seams the architecture doc already names, so consumers depend on what they use:

- ambient/context helpers (stdlib-only leaf)
- validation helpers (`_require_positive_int` and friends)
- frame/metadata shaping
- legacy query composition (used by `wqp`, `nldi`, `streamstats`, `nwis`)

Each move is independently testable and each should shrink `utils`'s Ca. The `.importlinter` contracts make the direction checkable at every step.

## What is deliberately *not* worth doing

Recording these so nobody "fixes" the score later by making the library worse:

- **Duplication 60/100.** All 8 clone groups are false positives: the per-collection getter families (thin, heavily documented wrappers whose bodies necessarily rhyme) and deliberate pairs like `utils.query`/`_query_with_retry` and `ChunkCall.partial_frame`/`partial_response`. Collapsing them trades the documented public surface for a metric.
- **19 "high-risk modules" / Modularity 50.** Driven by abstractness = 0 across all 50 modules, so every stable leaf sits at distance 1 from the main sequence ("zone of pain"). Raising it means adding ABCs to `exceptions`, `codes`, `rdb`, and `waterdata.types` — textbook metric gaming, and against this project's stated preference for functions over classes.
- **Architecture 85/100.** The 25 "violations" are mostly an SRP heuristic flagging `exceptions` and `credentials` for being imported by many packages. That is afferent coupling on a stable leaf, which is exactly what ADR 0003 asks for. Its DI check also misidentifies a function (`planning._extract_axes`) as a class.
- **"20 high-risk functions" (Complexity 95/100).** 17 of the 20 are synthetic `` rows where pyscn sums every function in a file — `planning.py` shows cognitive 140 with cyclomatic 1. Only 3 are real functions.

## Related: laddering the complexity ratchet

#360 refactored `ogc.filters._split_top_level_or` (extracting the OR-token lookahead) and tightened the `complexipy` gate from 36 to **27**. The next rungs, in order:

| function | cognitive |
|---|---|
| `nldi.get_features` | 27 |
| `ogc.requests._construct_api_requests` | 25 |
| `nwis._read_json` | 25 — *deprecated module, leave alone* |
| `ogc.planning.ChunkPlan._refine` | 22 |

Each one that comes down lets the gate tighten by the same amount, permanently. Worth doing opportunistically when touching those functions rather than as a dedicated refactor.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.