refactor(cog)!: reduce to_cog's 29-parameter signature via grouped option dataclasses (S107) - #986
Open
MAfarrag wants to merge 14 commits into
Open
refactor(cog)!: reduce to_cog's 29-parameter signature via grouped option dataclasses (S107)#986MAfarrag wants to merge 14 commits into
MAfarrag wants to merge 14 commits into
Conversation
…asses
Replace to_cog's 29 flat keyword arguments with six grouped, validated frozen
dataclasses exposed under the `cog` namespace — Compression, Overviews, Tiling,
BandSelection, Tags, and Layout — reducing the signature to nine parameters and
clearing SonarQube python:S107.
- `compression` accepts a profile-name string or a Compression (coerced),
collapsing the old profile/compress/level/quality/predictor knobs.
- Per-group validation now lives in each dataclass's __post_init__ (blocksize
power-of-2, quality range, predictor value, zoom strategy, band indices), so
invalid options fail at construction with a clear message.
- jpeg/webp dtype/band constraints now also apply when the compress method is
given directly, not only via a named profile.
- House write policy is unchanged: dtype-aware predictor, category-safe default
overview resampling, and the STATISTICS retry.
- Internal callers migrated: the CLI `cog create` command, grib_to_cog, and
to_file(driver="COG"). ~47 test call sites migrated to the grouped API.
BREAKING CHANGE: Dataset.to_cog(), to_cog_bytes(), and
DatasetCollection.to_cog_stack() no longer accept the flat COG option keyword
arguments (compress, blocksize, predictor, overview_resampling, tiling_scheme,
indexes, colormap, ...). Pass the grouped `cog.*` dataclasses instead — e.g.
`ds.to_cog("out.tif", compression="zstd", layout=cog.Layout(blocksize=256))`.
6 tasks
A direct Compression(compress="JPEG"/"WEBP") now passes straight to GDAL as the pre-refactor flat compress= did (GDAL accepts e.g. 4-band Byte JPEG); the dtype/band constraint fires only when the method was chosen via a profile string, restoring the prior behaviour and avoiding an input narrowing.
The COG driver's PREDICTOR option accepts YES/NO/STANDARD/FLOATING_POINT; 'NO' (disable the predictor) was rejected by the validator. Add it to the allowed set and document the full token list.
BandSelection/Tags carry list/dict fields, so instances are not hashable and their contents stay mutable — document that. Also drop the now-redundant quoted forward-ref annotations on Compression.coerce (from __future__ annotations).
grib.py and a test docstring still named the removed flat kwargs (to_cog(indexes=/target_srs=/profile=), compress="ZSTD"); point them at the grouped forms (bands=BandSelection, tiling=Tiling, compression=).
…e/blocksize paths Rename test_explicit_compress_overrides_profile (the to_cog profile+override interaction is gone) to an honest Compression(compress=...) assertion, and add CLI tests for 'cog create --profile deflate --compress ZSTD' (the replace() override branch) and '--blocksize 256'.
Direct unit tests for every __post_init__ branch (Compression quality/predictor, Overviews count, Tiling zoom strategy, BandSelection indices, Layout blocksize/bigtiff) and Compression.coerce (profile expansion, lerc max_z_error, None/identity/unknown-profile), closing the validator coverage gap.
_cmd_create coerced --profile to a Compression object, which set from_profile=False in to_cog and skipped the jpeg/webp dtype/band pre-check — a regression vs the Python compression="jpeg" path. Forward the profile string when no --compress override is given (build an object only for the override), and cover --profile jpeg on a float source with a test.
- L1: Compression accepts '1'/'2'/'3' (string) predictors the old flat API forwarded to GDAL; add a test. - L2: guard test that every PROFILES entry uses only the keys Compression.coerce carries (COMPRESS/LEVEL/QUALITY/MAX_Z_ERROR). - N1: clarify that the jpeg/webp dtype/band check applies to the profile-string form only, not a direct Compression object. - N2: make the hashability note precise (unhashable once a list/dict field is populated).
The overview mermaid diagram showed the removed compress="ZSTD" kwarg; use compression="zstd".
Both docs/examples/cog notebooks (run by the nbval notebooks CI) still called removed flat kwargs (profile=/compress=/tiling_scheme=/indexes=/band_tags=/...), which now raise TypeError. Migrate every cell to the cog.* grouped dataclasses; re-executed both headless clean (nbconvert, exit 0). Notebooks kept output-stripped.
docs/tutorials/cog.md documented the removed flat to_cog kwargs throughout; rewrite the snippets against compression=C:/Program Files/Git/overviews=/tiling=/bands=/tags=/layout=.
…S3776) The grouped-options rewrite pushed to_cog's cognitive complexity to 20 (>15). Move the dtype-aware predictor/overview resolution, the categorical guardrail, and the GDAL option-dict assembly into a _build_cog_defaults helper; behavior is unchanged (identical creation-option dict).
The grouped-options migration nested a dataclass constructor inside several pytest.raises/pytest.warns blocks, creating a second invocation (SonarCloud S5778/S9088). Hoist the construction out (or test the constructor directly where it is the thrower, e.g. Layout blocksize), and split one composite Tags assertion (S9073).
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Dataset.to_cog()accepted 29 keyword parameters, tripping SonarQubepython:S107(max 13). This groups thelow-level knobs into six typed, validated frozen dataclasses exposed under the
cognamespace, reducing thesignature to 9 parameters and clearing S107 as a real decomposition (Option B from the issue), not a linter
work-around.
New groups (
from pyramids.dataset import cog):cog.Compression—compress,level,quality,predictor,max_z_errorcog.Overviews—resampling,count,compresscog.Tiling—target_srs,resampling(warp),scheme,zoom_level,zoom_level_strategy,aligned_levelscog.BandSelection—indexes,out_dtype,nodatacog.Tags—band_tags,colormap,metadatacog.Layout—blocksize,bigtiff,num_threads,add_mask,sparse_ok,statisticsNew signature:
Highlights:
compressionaccepts a profile-name string or aCompression(coerced viaCompression.coerce), collapsingthe old
profile/compress/level/quality/predictorknobs into one concept. The common call stays terse:ds.to_cog("out.tif", compression="zstd").__post_init__(blocksize power-of-2, quality range, predictorvalue, zoom strategy, band indices), so invalid options fail at construction with a clear message.
jpeg/webpdtype/band constraints now also apply when the compress method is given directly, not only via a namedprofile (a strictly clearer, earlier error).
STATISTICSretry — identical COG output for equivalent inputs.cognamespace (not hoisted to top-levelpyramids), so generic names likeCompressionstay free for other APIs;
BandSelectionavoids the existingengines.Bandscollision.Internal callers migrated: the
pyramids cog createCLI command,grib_to_cog, andto_file(driver="COG").write_cogwas alreadyextra=-based and is unaffected.Issues
to_cog's 29-parameter signature (python:S107).to_cog's 29 parameters into typedcog.*option dataclasses.Type of change
Check relevant points.
How Has This Been Tested?
Targeted COG suites re-run locally against the dev environment:
584 passed, 0 failedacross the COG write/validate/inspect suites,to_cog_stack, GRIB, and theNetCDF-inherited-ops sample.
--doctest-modules src/pyramids/dataset/cog/options.py src/pyramids/dataset/engines/cog.py src/pyramids/dataset/cog/facade.py src/pyramids/cli.py→ 11 passed, 12 skipped).compression="zstd"string coercion + dtype cast + all three validatorsfiring; package imports with no cycles.
Checklist: