Skip to content

refactor(cog): reduce to_cog's 29-parameter signature (python:S107) #757

Description

@MAfarrag

Context

python:S107 flags COG.to_cog (src/pyramids/dataset/engines/cog.py:167) for having 29 parameters (limit 13).
It's the only one of the 7 open S107 issues worth a design conversation — the rest are sibling flat factories
(from_stac, create_from_array, plot facades) that follow the same intentional pattern accepted on #631.

This is a non-gating maintainability smell. This issue captures a researched decision on whether/how to reduce the
count.

Framing update — pyramids is pre-1.0 / storming phase, so a breaking signature change is acceptable. With the
backward-compat tax removed, the analysis below re-ranks the options: the deprecation-shim requirement (previously
PEP 692's biggest edge) is no longer decisive, and the internal 133 call sites + CLI + doctests are simply updated as
part of the change. External-user breakage is the only residual concern, mitigated by a CHANGELOG/migration note.
See "Design-merit comparison (breaking cost excluded)" for the head-to-head that drives the recommendation.

Current signature (29 kw-only params)

path + profile, compress, level, quality, blocksize, predictor, bigtiff, num_threads, overview_resampling, overview_count, overview_compress, tiling_scheme, zoom_level, zoom_level_strategy, aligned_levels, resampling, add_mask, sparse_ok, target_srs, statistics, indexes, out_dtype, nodata, band_tags, colormap, metadata, config, extra.

How the ecosystem structures this (research)

Library Function Named params Low-level GDAL creation options via
pyramids Dataset.to_cog 29 flat named and extra/profile/config
rio-cogeo cog_translate ~25 a single dst_kwargs: Dict + config: Dict
rioxarray rio.to_raster ~8 **profile_kwargs
rasterio open("w", …) 0 fixed a profile dict / **kwargs
GDAL gdal.Translate (COG) creationOptions: list[str]

Takeaway: every peer funnels the low-level creation options through a dict / **kwargs / options-list and only
names the high-level knobs. pyramids uniquely enumerates ~12 low-level GDAL options as flat named params on top of
already exposing extra/profile/config. (rio-cogeo is still ~25 params though — feature-rich COG writers inherently
carry many knobs.)

Side-effect audit (what a signature change actually touches)

Surface Detail Impact of a breaking signature change
Callers 133 .to_cog(...) sites in src+tests; external users unknown high blast radius
Dataset.to_cog already a *args/**kwargs facade (dataset.py:384) unaffected — good
to_cog_bytes def to_cog_bytes(self, **kwargs) forwards to to_cog unaffected if kwargs still accepted
write_cog facade calls ds.to_cog(output, extra=options or None) — only uses extra= insulated
CLI cli.py:139 builds a dict and does ds.to_cog(args.output, **kwargs) grouped dataclasses ⇒ new mapping code; flat/**kwargs ⇒ unchanged
Doctests to_cog docstring has 10 doctests using flat kwargs grouped dataclasses break them; flat kwargs keep them
Docs 46-row Args table in the docstring grouped ⇒ fragments across 5 dataclass docstrings

Real usage is concentrated: across all in-repo callers only ~16 distinct kwargs ever appear, dominated by profile,
blocksize, compress, extra, indexes, predictor, overview_resampling, colormap. The other ~13 params are
completeness/passthrough.

Options considered

A — Accept / Won't-Fix (no code change)

Mark the issue Accepted in SonarCloud with a rationale (deliberate, ergonomic, ecosystem-comparable flat API).

B — Group into typed option dataclasses (5 clusters) — recommended (pre-1.0), see verdict ⭐

CogCompression, CogOverviews, CogBands, CogTags, CogWrite (+ target_srs) → signature drops to 7 params.
Add scalar coerce on the hot fields (compression: str | CogCompression) to keep the common case terse.

  • + validated, reusable, IDE-discoverable typed groups with native defaults + __post_init__ validation;
    extends the existing cog/options.py::CreationOptions pattern; real decomposition (clears S107 in spirit).
  • breaking for the 133 callers + CLI + 10 doctests (all migrated as part of the change — acceptable pre-1.0,
    no deprecation shim); fragments the 46-row Args table across the dataclass docstrings; slightly more verbose for
    advanced multi-group calls (the coerce shorthand covers the common case).

C — PEP 692 **kwargs: Unpack[CogOptions] (fallback — the non-breaking alternative)

Replace the 29 flat params with def to_cog(self, path, **options: Unpack[CogOptions]) -> Path, where CogOptions
is a TypedDict carrying all fields as NotRequired. Declared params drop to ~3 → clears S107.

  • + zero breaking — identical flat call syntax, so all 133 callers, the CLI's **kwargs dict, to_cog_bytes,
    and the 10 doctests are untouched; adds full type-checking + IDE autocomplete via the TypedDict; the codebase
    already uses Unpack/TypedDict (feature/geometry.py); works on the 3.11 floor (typing_extensions.Unpack).
  • TypedDict fields cannot carry defaults — defaults move into the body (options.get("blocksize", 512)),
    a readability/boilerplate cost; mkdocstrings/griffe has known quirks rendering Unpack[TypedDict] params
    (griffe issues ci(tests): Phase 12 — per-extras CI matrix + shared _marks module + testing how-to + env teardown fix #284 / PR fix(wheels): vendor curl CA bundle so vendored-GDAL HTTPS reads work (#412) #414) — the docs build must be verified; and some reviewers may see reducing declared
    params while keeping 29 effective options as skirting the rule's intent (counter: a typed **kwargs is more
    readable + safer than 29 flat params, so it's a legitimate resolution, not a hack).

D — Hybrid: keep the ~8 common options named, fold the rare ~20 into Unpack/extra

Lands ~13 named params + typed kwargs — best ergonomics for common paths, but adds design complexity and may still sit
at the S107 boundary. Consider only if C's defaults-in-body cost is deemed too high.

Design-merit comparison (breaking cost excluded)

With backward-compat off the table (pre-1.0), PEP 692's advantages collapse to essentially call-site terseness +
familiarity
, while dataclass groups win the things that compound over a library's life:

Dimension PEP 692 **kwargs: Unpack[CogOptions] Dataclass groups Winner
Defaults ❌ TypedDict can't hold defaults → live in the body (opts.get("blocksize", 512)), split from the type ✅ native (blocksize: int = 512); the type is the spec dataclass
Runtime validation ❌ none (bare dict) → scattered in the body __post_init__ co-locates it (blocksize power-of-2, profile/compress conflicts) dataclass
S107 in spirit ⚠️ satisfies the letter — 29 options relocate into the TypedDict ✅ real decomposition into 5 single-responsibility types dataclass
Conceptual structure ❌ one flat bag of 29 keys ✅ groups teach the domain (compression / overviews / bands / tags / write) dataclass
Docs tooling ❌ mkdocstrings/griffe quirks rendering Unpack[TypedDict] (#284/#414) ✅ each dataclass renders cleanly; signature is tidy dataclass
Reusable/serializable option objects ⚠️ a typed dict you **-splat ✅ frozen, hashable, storable in config, replace()-able dataclass
Testability ❌ nothing to unit-test ✅ construction/validation testable in isolation dataclass
Common-case terseness to_cog(p, compress="ZSTD", blocksize=256) to_cog(p, compression=CogCompression(compress="ZSTD"), overviews=CogOverviews(blocksize=256)) PEP 692
Familiarity (Pythonic) ✅ flat kwargs are expected ⚠️ option-objects are less common PEP 692

Verdict: once breaking is acceptable, dataclass grouping is the stronger long-term design. PEP 692 was mostly
winning because it was the cheap, non-breaking move.

Closing the terseness gap — coercion on the hot fields

The dataclass approach's one real weakness (ceremony for the common case) is fixable without giving up structure:
accept a scalar shorthand OR the full object via a union + a coerce classmethod.

def to_cog(
    self, path, *,
    compression: str | CogCompression | None = None,   # "zstd" | "deflate" | CogCompression(...)
    overviews: CogOverviews | None = None,
    bands: CogBands | None = None,
    tags: CogTags | None = None,
    write: CogWrite | None = None,
    target_srs: int | str | None = None,
) -> Path:
    compression = CogCompression.coerce(compression)   # str -> profile preset; None -> defaults

Common case becomes to_cog(path, compression="zstd") — as terse as flat kwargs — while power users still get the full
typed CogCompression(compress="LERC", level=9, predictor=3). This recovers ~90% of PEP 692's ergonomics edge and keeps
every dataclass advantage. It also reads as a nicer API than rasterio's opaque **kwargs — a differentiator, not a
regression.

Recommendation

Given pyramids is pre-1.0 (breaking acceptable):

  1. Adopt Option B — dataclass groups with scalar-coercion on the hot fields (compression, likely target_srs).
    This is the best long-term design: real decomposition, native defaults, co-located validation, clean docs, reusable
    option objects — with the coercion neutralizing the only reason to prefer PEP 692. Update the 133 call sites, the
    CLI, and the 10 doctests as part of the change; add a CHANGELOG migration note for external users.
  2. Option C (PEP 692 Unpack) is the fallback only if the team decides flat-kwargs terseness/familiarity is a
    hard product requirement, or wants a strictly non-breaking change after all.
  3. Option A (Accept / Won't-Fix) remains the zero-effort default if we choose not to invest now.

Out of Scope

  • The other 6 S107 signatures (accept as intentional flat factories/plot facades).
  • The S3776 cognitive-complexity tier and the rest of the backlog
    (planning/cleaning/sonar-main-remaining-2026-07-14.md).

Effort Estimate

Size: B (recommended) = M (5 dataclasses + coerce + migrate 133 callers/CLI/10 doctests — no deprecation shim
needed since breaking is acceptable) · C = S (TypedDict + body defaults + docs verification) · A = XS
(a SonarCloud transition).

Definition of Done

  • Decision recorded (B / C / A) with rationale
  • If B (recommended): CogCompression/CogOverviews/CogBands/CogTags/CogWrite dataclasses added with
    native defaults + __post_init__ validation; coerce on the union-typed hot fields (compression,
    target_srs); the 133 call sites, the CLI (cli.py), and the 10 to_cog doctests migrated to the new signature;
    to_cog_bytes still forwards; write_cog (extra=) unaffected; a CHANGELOG migration note for external users;
    full non-plot suite green; S107 clears on the next analysis
  • If C: CogOptions TypedDict added; body applies defaults; type-checks on 3.11/3.12; doctests still green;
    mkdocstrings renders the params (or a documented workaround); S107 clears
  • If A: to_cog's S107 marked Accepted in SonarCloud with the documented reason
  • No behaviour change to the emitted COG for equivalent inputs

Sources: PEP 692 (Unpack[TypedDict] for **kwargs) ·
rio-cogeo cog_translate source ·
rioxarray to_raster ·
rasterio profiles & writing ·
griffe TypedDict-kwargs handling (#284 / PR #414) ·
PEP 702 @deprecated

Metadata

Metadata

Assignees

No one assigned

    Labels

    devinstallation, cienhancementNew feature or requestpythonPull requests that update python code

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions