DOI-USGS / DOI-USGS/dataretrieval-python

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

オープン
#361 コメント 1 件 リアクション 0 件 担当者 0 名 GitHub で見る
主要言語
Python
スター
266
フォーク
63
平均マージ
1日 20時間
マージ済み PR(30日)
19

説明

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.

コントリビューションガイド

コントリビューションガイドを開く

調査の方向性

まず utils.py、ogc/context.py、ogc/chunking.py から始め、次に依存関係のない Ambient wrapper を dataretrieval/ambient.py に移動し、その2か所の import を更新します。.importlinter のレイヤースタックを読み、最初にそのコントラクトを実行します。完了とは、import がパスし、文書化された動作を変更せずに、より広い utils.py の境界が分離されていることです。

索引モデルが issue の本文から書いたものです。

評価

技術スタック
python
領域
backend, tooling
issue の種類
リファクタリング
難易度
4/5
見積もり時間
3〜5日
活発さ
静か
明瞭さ
おおむね明確
初心者へのやさしさ
52/100

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。