geotiff: extract writers to _writers/, reduce __init__.py to dispatch (#1888)#1899
Merged
Conversation
…#1888) Final step (10) of #1813's multi-PR refactor of __init__.py. Pure code motion; no public API change. Created the _writers/ subpackage with three modules: - _writers/eager.py: ``to_geotiff`` (public eager writer), ``_write_single_tile`` (per-tile worker), and ``_write_vrt_tiled`` (deprecated vrt_tiled=True path). ~870 lines. - _writers/gpu.py: ``write_geotiff_gpu`` (nvCOMP-backed GPU writer with CPU fallback). ~500 lines. - _writers/vrt.py: ``write_vrt`` (mosaic XML generator with crs_wkt deprecation handling). ~90 lines. __init__.py re-exports the three public writer names. The two private helpers tests reach for (``_write_single_tile``, ``_apply_nodata_mask_gpu``) keep their package-level re-exports with ``# noqa: F401`` markers since the test files import them directly. Final shape of __init__.py: 529 lines (down from 1905 pre-PR and 4902 at the start of #1813 series). It now holds only: - Module docstring + ``__all__`` (lines 1-112). - ``_read_geo_info``: metadata-only header parse used by ``open_geotiff`` and lazy-imported by ``_backends/dask.py``. - ``open_geotiff``: the public auto-dispatching read entry point that routes between the four read backends and ``read_vrt``. - ``plot_geotiff``: deprecated wrapper that emits a ``DeprecationWarning`` pointing to ``da.xrs.plot()``. Test updates ============ - test_to_geotiff_gpu_fallback_1674.py monkeypatches ``write_geotiff_gpu`` and ``_is_gpu_data`` as resolved by ``to_geotiff``. The patch targets move from ``xrspatial.geotiff`` to ``xrspatial.geotiff._writers.eager`` so the spy still intercepts the real call sites. Same module-global-rebind pattern as #1895 and #1897. Net change since the start of #1813 ==================================== - __init__.py: 4902 -> 529 lines (-4373; -89%). - New per-responsibility modules under xrspatial/geotiff/: _runtime.py (#1880), _crs.py (#1881), _validation.py (#1882), _attrs.py (#1883), _backends/_gpu_helpers.py (#1884), _backends/gpu.py (#1885), _backends/dask.py (#1886), _backends/vrt.py (#1887), _writers/{eager,gpu,vrt}.py (this PR). - One new regression-backstop test: test_backend_pixel_parity_matrix_1813.py (#1879), 192 parametrised cells covering pixel/coord/attrs parity across the four backends and four read entry points. Verification: 2862/2862 of the geotiff test suite pass (the 8 remaining pre-existing main failures from #1517 and #1776 are unrelated to #1813 and were excluded as deselected throughout). Closes #1888. Closes #1813.
Contributor
There was a problem hiding this comment.
Pull request overview
Step 10 (final) of issue #1813. Pure code motion: lifts to_geotiff, _write_single_tile, _write_vrt_tiled, write_geotiff_gpu, and write_vrt out of xrspatial/geotiff/__init__.py into a new _writers/ subpackage, leaving __init__.py as a thin dispatch module re-exporting the public writer names. Function bodies are unchanged aside from relative-import adjustments.
Changes:
- New
_writers/eager.py,_writers/gpu.py,_writers/vrt.pymodules holding the writer entry points;_writers/__init__.pyis intentionally empty (just a docstring). __init__.pyshrinks from ~1900 → ~529 lines by removing the writer bodies and re-exporting their names (with_write_single_tilekept re-exported under# noqa: F401for tests).test_to_geotiff_gpu_fallback_1674.pyretargets itswrite_geotiff_gpu/_is_gpu_datamonkeypatches from the package namespace toxrspatial.geotiff._writers.eagerso the spies still hit the real call sites.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| xrspatial/geotiff/init.py | Removes the writer function bodies; adds re-imports from the new _writers/ modules. |
| xrspatial/geotiff/_writers/init.py | New empty package docstring module. |
| xrspatial/geotiff/_writers/eager.py | New module hosting to_geotiff, _write_single_tile, _write_vrt_tiled. |
| xrspatial/geotiff/_writers/gpu.py | New module hosting write_geotiff_gpu. |
| xrspatial/geotiff/_writers/vrt.py | New module hosting the public write_vrt wrapper with crs_wkt deprecation. |
| xrspatial/geotiff/tests/test_to_geotiff_gpu_fallback_1674.py | Updates monkeypatch targets to the writer's new defining module. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+13
to
+50
| import math | ||
| import os | ||
| import warnings | ||
| from typing import TYPE_CHECKING | ||
|
|
||
| import numpy as np | ||
| import xarray as xr | ||
|
|
||
| if TYPE_CHECKING: | ||
| from typing import BinaryIO | ||
|
|
||
| from .._attrs import ( | ||
| _LEVEL_RANGES, | ||
| _VALID_COMPRESSIONS, | ||
| _extract_rich_tags, | ||
| _resolve_nodata_attr, | ||
| ) | ||
| from .._backends._gpu_helpers import _is_gpu_data | ||
| from .._coords import ( | ||
| _BAND_DIM_NAMES, | ||
| coords_to_transform as _coords_to_transform, | ||
| transform_from_attr as _transform_from_attr, | ||
| ) | ||
| from .._crs import _wkt_to_epsg | ||
| from .._geotags import GeoTransform, RASTER_PIXEL_IS_AREA, RASTER_PIXEL_IS_POINT | ||
| from .._runtime import ( | ||
| GeoTIFFFallbackWarning, | ||
| _geotiff_strict_mode, | ||
| _gpu_fallback_warning_message, | ||
| ) | ||
| from .._validation import ( | ||
| _validate_3d_writer_dims, | ||
| _validate_dtype_cast, | ||
| _validate_tile_size_arg, | ||
| ) | ||
| from .._writer import write | ||
| from .gpu import write_geotiff_gpu | ||
| from .vrt import write_vrt |
Comment on lines
+10
to
+35
| import math | ||
| import os | ||
| import warnings | ||
| from typing import TYPE_CHECKING | ||
|
|
||
| import numpy as np | ||
| import xarray as xr | ||
|
|
||
| if TYPE_CHECKING: | ||
| from typing import BinaryIO | ||
|
|
||
| from .._attrs import _extract_rich_tags, _resolve_nodata_attr | ||
| from .._coords import ( | ||
| _BAND_DIM_NAMES, | ||
| coords_to_transform as _coords_to_transform, | ||
| transform_from_attr as _transform_from_attr, | ||
| ) | ||
| from .._crs import _wkt_to_epsg | ||
| from .._geotags import GeoTransform | ||
| from .._runtime import GeoTIFFFallbackWarning | ||
| from .._validation import ( | ||
| _validate_3d_writer_dims, | ||
| _validate_dtype_cast, | ||
| _validate_tile_size_arg, | ||
| ) | ||
| from .._writer import write |
Drop the unused module-level and in-function imports Copilot flagged: eager.py - Drop ``import math`` (no math.* calls in the file). - Drop ``RASTER_PIXEL_IS_POINT`` re-import (only RASTER_PIXEL_IS_AREA is referenced; the POINT path lives in _writers/_vrt_tiled below and uses its own internal constants). - Drop ``_validate_dtype_cast`` re-import (never called; the writer side does not validate dtype casts post-decode). - Drop ``from .vrt import write_vrt`` re-import; ``_write_vrt_tiled`` does a lazy ``from .._vrt import write_vrt as _write_vrt_fn`` at its single call site so the module-top import was dead. - Drop the redundant ``import os`` inside ``_write_vrt_tiled``; the module-level ``import os`` already covers the os.path / os.makedirs calls in that body. gpu.py - Drop ``import math`` and ``import os`` (no math.* / os.* references in the file body). - Drop ``GeoTransform`` re-import at module level (only used in eager.py's _write_single_tile, which still imports it). - Drop ``_validate_dtype_cast`` re-import (never called). - Drop ``from .._writer import write`` re-import; only docstring references mention it. - Drop the in-function ``GeoTransform as _GT`` alias inside the ``_writer`` import block (the alias was carried over verbatim from __init__.py but the function body never references _GT). - Drop the in-function ``from .._dtypes import numpy_to_tiff_dtype`` (also unreferenced in the body). All 2862 geotiff tests still pass; pyflakes is clean on these files except for the ``cupy.ndarray`` type annotation in ``write_geotiff_gpu``'s signature, which works fine under ``from __future__ import annotations`` and matched the same surface the original __init__.py had through earlier #1813 PRs.
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
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.
Summary
Final step (10) of #1813's multi-PR refactor of
xrspatial/geotiff/__init__.py. Pure code motion; no public API change.Created the
_writers/subpackage with three modules:_writers/eager.py—to_geotiff(public eager writer),_write_single_tile(per-tile worker),_write_vrt_tiled(deprecatedvrt_tiled=Truepath). ~870 lines._writers/gpu.py—write_geotiff_gpu(nvCOMP-backed GPU writer with CPU fallback). ~500 lines._writers/vrt.py—write_vrt(mosaic XML generator withcrs_wktdeprecation handling). ~90 lines.__init__.pyre-exports the three public writer names. The two private helpers tests reach for (_write_single_tile,_apply_nodata_mask_gpu) keep their package-level re-exports with# noqa: F401markers since the test files import them directly.Final shape of
__init__.py529 lines (down from 1905 pre-PR and 4902 at the start of #1813). It now holds only:
__all__(lines 1-112)._read_geo_info: metadata-only header parse used byopen_geotiffand lazy-imported by_backends/dask.py.open_geotiff: the public auto-dispatching read entry point that routes between the four read backends andread_vrt.plot_geotiff: deprecated wrapper that emits aDeprecationWarningpointing toda.xrs.plot().Test updates
test_to_geotiff_gpu_fallback_1674.pymonkeypatcheswrite_geotiff_gpuand_is_gpu_dataas resolved byto_geotiff. The patch targets move fromxrspatial.geotifftoxrspatial.geotiff._writers.eagerso the spy still intercepts the real call sites. Same module-global-rebind pattern as #1895 and #1897.Net change since the start of #1813
__init__.py: 4902 → 529 lines (-4373; -89%)._runtime.py(geotiff: extract sentinels, fallback warning, and strict-mode helper into _runtime.py (#1813 step 2) #1880),_crs.py(geotiff: extract CRS resolution helpers into _crs.py (#1813 step 3) #1881),_validation.py(geotiff: extract input validators into _validation.py (#1813 step 4) #1882),_attrs.py(geotiff: extract attrs/metadata helpers into _attrs.py (#1813 step 5) #1883),_backends/_gpu_helpers.py(geotiff: extract GPU helpers into _backends/_gpu_helpers.py (#1813 step 6) #1884),_backends/gpu.py(geotiff: extract read_geotiff_gpu into _backends/gpu.py (#1813 step 7) #1885),_backends/dask.py(geotiff: extract read_geotiff_dask into _backends/dask.py (#1813 step 8) #1886),_backends/vrt.py(geotiff: extract read_vrt and VRT dask helpers into _backends/vrt.py (#1813 step 9) #1887),_writers/{eager,gpu,vrt}.py(this PR).test_backend_pixel_parity_matrix_1813.py(geotiff: add end-to-end pixel-byte-parity test matrix (#1879) #1889) — 192 parametrised cells covering pixel/coord/attrs parity across the four backends and four read entry points.Test plan
Closes #1888. Closes #1813.