Skip to content

Synthesize x/y coords for no-georef VRT reads#2824

Merged
brendancol merged 3 commits into
mainfrom
issue-2818
Jun 2, 2026
Merged

Synthesize x/y coords for no-georef VRT reads#2824
brendancol merged 3 commits into
mainfrom
issue-2818

Conversation

@brendancol

Copy link
Copy Markdown
Contributor

Closes #2818

A VRT with no <GeoTransform> element came back with no x/y coords on both the eager and chunked read paths, while a plain GeoTIFF with no georef tags synthesizes integer pixel coords 0..N-1 (via coords_from_pixel_geometry(..., has_georef=False) in _coords.py). Downstream code that assumes x/y always exist broke on no-georef VRTs but worked on the equivalent plain GeoTIFF.

What changed

  • Both VRT paths now reuse the shared coords_from_pixel_geometry(has_georef=False) helper in the no-<GeoTransform> branch (eager: _backends/vrt.py ~line 561; chunked: ~line 1098) instead of leaving coords = {}.
  • The result carries integer x/y coords with georef_status='none', matching the non-VRT path. Windowed reads shift the coords to the window offset, same as the non-VRT windowed read.

Backend coverage

The change touches the eager (numpy) and chunked (dask) VRT read paths. GPU reads go through the same chunked code, so the synthesized coords apply there too. No backend support changed.

Test plan

  • New parity tests assert x/y coords exist and match the non-VRT synthesized coords for eager, chunked, and windowed no-georef VRT reads
  • Confirmed the new tests fail without the fix (empty coords) and pass with it
  • Full VRT suite: 494 passed
  • Full geotiff suite: 6052 passed, 81 skipped, 1 xfailed
  • flake8 clean on changed files

A VRT with no <GeoTransform> element came back with no x/y coords on
both the eager and chunked read paths, while a plain GeoTIFF with no
georef tags synthesizes integer pixel coords 0..N-1 via _coords.py.
Downstream code that assumes x/y exist broke on no-georef VRTs.

Both VRT paths now reuse coords_from_pixel_geometry(has_georef=False)
in the no-GeoTransform branch, so they emit the same integer x/y
arrays as the non-VRT path (windowed reads shift to the window
offset). georef_status stays 'none'.

Adds parity tests asserting x/y coords exist and match the non-VRT
synthesized coords for eager, chunked, and windowed reads.
@github-actions github-actions Bot added the performance PR touches performance-sensitive code label Jun 2, 2026

@brendancol brendancol left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Review: Synthesize x/y coords for no-georef VRT reads

Blockers

None.

Suggestions

None blocking.

Nits

  • _backends/vrt.py, eager else-branch (~610): the window/height/width setup repeats the georef branch right above it. The chunked path hoists coord_window out of the if/else; the eager path could do the same and drop the second arr.shape[:2] read. Cosmetic. The two branches are mutually exclusive and the duplication is tiny.

What looks good

  • Both fixed branches call the shared coords_from_pixel_geometry(has_georef=False) helper instead of re-deriving the arange logic, so the no-georef coord convention stays in one place.
  • Windowed reads work: the eager branch reuses the georef branch's max(0, window[0]) offset, and the chunked branch reuses (win_r0, win_c0, win_r0+full_h, win_c0+full_w), which matches the non-VRT dask path's (win_r0, win_c0, win_r1, win_c1).
  • Tests check coord values and dtype against the non-VRT reference for eager, chunked, and windowed reads, and they fail without the fix.

Checklist

  • Coords match non-VRT reference (values + dtype)
  • georef_status stays 'none'
  • Dask (chunked) path covered
  • Windowed reads covered
  • No premature materialization (coords built from extents, not data)
  • No new public API; README/notebook not applicable

The eager no-georef branch repeated the height/width and coord_window
computation from the georef branch. Hoist both above the if/else so the
two branches share them, matching the chunked path's structure.

@brendancol brendancol left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Review (follow-up)

The nit from the first pass is resolved: the eager path now hoists height/width and coord_window above the if gt is not None block, so the georef and no-georef branches share the same setup. This matches the chunked path's structure.

Blockers

None.

Suggestions

None.

Nits

None.

Both VRT read paths synthesize integer x/y pixel coords for no-GeoTransform VRTs via the shared coords_from_pixel_geometry(has_georef=False) helper, matching the non-VRT path. Tests cover eager, chunked, and windowed reads and fail without the fix. 494 VRT tests pass, flake8 clean.

@brendancol brendancol merged commit 3e1980f into main Jun 2, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

performance PR touches performance-sensitive code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

VRT no-georef reads drop x/y coordinates entirely

1 participant