Skip to content

Preserve dimensions for empty orthogonal indexing - #1499

Merged
jswhit merged 5 commits into
Unidata:masterfrom
zack-dev-cm:fix/empty-index-dimensions
Oct 7, 2026
Merged

jswhit merged 5 commits into
Unidata:masterfrom
zack-dev-cm:fix/empty-index-dimensions

Conversation

@zack-dev-cm

@zack-dev-cm zack-dev-cm commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Empty integer indices and all-false masks can collapse surviving dimensions: an empty selection on a (24, 100) variable returns (0, 1) instead of (0, 100). This reproduces in netCDF4 1.7.4 and explains the backend behavior reported in pydata/xarray#11601.

Preserve selected lengths and scalar squeeze markers when an orthogonal selection is empty, without constructing a Cartesian product of fancy indices. Skip native reads for empty output, preserve automatic decoding and character-conversion eligibility, and keep broadcastable empty writes as no-ops. Multi-file empty reads derive their decoded dtype from the selected files.

The final diff contains the two indexing implementation files, test/test_empty_indexing.py, and the requested Changelog entry. Upstream master is integrated, including its benchmark typing, numeric-attribute byte-order and command-line corrections. The indexing correction and regression edits are unchanged from the previously validated revision; complete source files also contain those upstream changes.

Regressions cover scalar/slice/array/mask combinations, orthogonal indexing, strings and variable-length data, unlimited dimensions, empty writes, bounded metadata, and multi-file packing/unsigned conversion.

Validation at final head 9a6bc82405d3ef53e704e6b45e71796e2d242f61:

  • Four upstream workflows succeed: Build and Test, development C library, latest C library, and Wheels. The rollup reports 42 successful checks and the tag-only publication skip.
  • The two Python 3.14 native C-library jobs each run 133 serial tests, with five and four skips respectively. Both workflows also complete their configured MPI checks.
  • The Windows ARM64 installed-wheel job tests Python 3.11, 3.12, 3.13, 3.14, 3.14t, 3.15 and 3.15t. Each profile passes 148 tests and 11,245 subtests, with eight skips.
  • The older C-library workflow ends in a GitHub Actions startup failure before creating any jobs. Its page reports an unexpected Actions error; it supplies no final-head test result for that profile. Passing checks and the merge do not establish complete dependency coverage.

Earlier defect validation used a rebuilt wheel installed outside the checkout on macOS ARM64, Python 3.12.3, NumPy 2.5.3, netCDF-C 4.9.3 and HDF5 1.14.6:

  • All 14 regressions and 863 subtests pass; the unchanged baseline has 396 failing and 467 passing subtests.
  • The bounded broader suite passes 123 tests with five skips, plus all 863 subtests.
  • Unittest discovery, regression-module typing, 12 direct/backend probes, and six synthetic Xarray-to-Zarr roundtrips pass.
  • An 8,000-case helper comparison preserves baseline nonempty metadata and exceptions; it does not exercise native I/O.

For the indexing review, _StartCountStride retains one compact chunk when any selected axis is empty, so surviving lengths and scalar squeeze markers remain available. Nonempty selections still expand to the existing orthogonal chunk grid. The empty MFDataset path retains selected-file decoding: slices promote the contributing files' decoded dtypes, non-slice record chunks retain their final record's dtype, and a selection with no records follows the first file's convention. Tests exercise these behaviors with packed/unsigned data and character conversion as well as ordinary numeric variables.

Retain selected dimensions and scalar squeeze markers without allocating Cartesian chunk grids for empty results. Preserve broadcastable empty writes and automatic decoding, using the selected multifile components and existing slice/fancy-index dtype rules.
@CLAassistant

CLAassistant commented Sep 24, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@zack-dev-cm

zack-dev-cm commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor Author

The current head a0a3271 includes upstream master and #1501, with only the three indexing files in the PR diff. The benchmark is identical to upstream. All three current-head contributor Python 3.14 workflows pass: development netCDF-C, netCDF-C 4.7.4, and netCDF-C 4.9.3. CLA passes; the five upstream workflows still await maintainer approval. Current-head Windows ARM64 execution remains unverified.

@jswhit

jswhit commented Oct 2, 2026

Copy link
Copy Markdown
Collaborator

Thanks for detecting and fixing this. I'm a bit concerned about the amount and opacity of code that the agent created to solve the problem - however, the indexing code is already pretty opaque so perhaps I shouldn't be surprised.

@jswhit

jswhit commented Oct 6, 2026

Copy link
Copy Markdown
Collaborator

Please create a Changelog entry (after merging updates from master).

Sync upstream 7b4d837 as requested for Unidata#1499. Preserve the existing contribution and history; no unrelated feature changes.
Document preserved dimensions and decoded types after merging current upstream master. This commit changes only the requested release note.
@jswhit
jswhit merged commit ba22b9c into Unidata:master Oct 7, 2026
43 checks passed
@zack-dev-cm

Copy link
Copy Markdown
Contributor Author

Thanks for reviewing and merging this, @jswhit. I appreciate your guidance on the implementation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants