Skip to content

Keep explicit real variable shapes with auto_complex=True - #1525

Open
FFspiky wants to merge 2 commits into
Unidata:masterfrom
FFspiky:fix/1404-auto-complex-write
Open

FFspiky wants to merge 2 commits into
Unidata:masterfrom
FFspiky:fix/1404-auto-complex-write

Conversation

@FFspiky

@FFspiky FFspiky commented Nov 11, 2026

Copy link
Copy Markdown

With auto_complex=True, creating an explicitly real variable over a length-two complex dimension currently hides that dimension while retaining the real dtype. Writing a matching real array then fails with a reshape/broadcast error.

Use the variable's Python dtype to select complex rank, dimension and I/O handling. Explicitly created real and compound variables retain their requested dimensions and storage types. Reopening a supported complex layout with auto_complex=True still produces the complex dtype and hides its component axis. Document this creation/reopening distinction.

Fixes #1404.

Regression coverage includes the three component-dimension names, f4/f8/i4 storage, contiguous/strided/reversed reads and writes, NETCDF4/NETCDF4_CLASSIC/NETCDF3_CLASSIC real storage, actual compound/dimension complex storage at c8/c16 precision, casts from real input, and reopening for complex reads and writes.

Validation on macOS arm64, Python 3.12.14, NumPy 2.5.4, netcdf-c 4.9.3 and HDF5 1.14.6, using a native editable build of the changed source:

  • The original example fails on unmodified master and succeeds after the patch; new real-layout cases also fail against the unmodified native extension.
  • pytest test/test_complex.py -q: 9 passed, 33 subtests passed.
  • pytest test --ignore=test/test_dap.py -q: 160 passed, 1 skipped, 1 xpassed, 11,288 subtests passed. The xpass is the existing blosc-marked case.
  • An additional native audit passed 31 cases covering strings, vlen arrays, enum/nested compound types, unlimited dimensions and chunking.
  • git diff --check passed.

OPeNDAP, MPI, other platforms and native sanitizers were not run locally. Existing NETCDF4_CLASSIC complex creation and other native convention-detection limitations are outside this fix.

AI assistance: OpenAI Codex implemented the patch and tests, built and tested the native extension, and drafted this description.

Select complex dimension and I/O handling using the variable dtype, preserving automatic complex decoding on reopen.

Fixes Unidata#1404.

Assisted-by: Codex
@jswhit jswhit self-assigned this Nov 12, 2026
@jswhit

jswhit commented Nov 12, 2026

Copy link
Copy Markdown
Collaborator

@ZedThree could you review this?

@jswhit

jswhit commented Nov 12, 2026

Copy link
Copy Markdown
Collaborator

@FFspiky this needs a Changelog entry

@FFspiky

FFspiky commented Nov 12, 2026 •

Copy link
Copy Markdown
Author

@jswhit Added the entry under the unreleased 1.7.6 section in 56ca59b. This follow-up only changes Changelog; the focused complex suite passes locally (9 tests, 33 subtests), and git diff --check passes.

I also checked the two failing jobs from the previous revision:

  • MPI: the baseline job on 0a79d16 already has the same zstd undefined-filter error. Its green result masks that error: run_all.py exits unsuccessfully, but bash -l {0} continues and the later MPI command succeeds. A standalone native probe on my fork, with the same HDF5/netCDF-C/OpenMPI/UCX package builds and no Python extension loaded, reproduces the filter error. The PR's additional MPI crash follows a UCX ud_verbs/mana_0:1 initialization failure and occurs in nc_create_par, before the changed variable handling. The native MPI probe passes with both default and TCP transports, but this fresh runner uses Ubuntu 26.04 rather than the original 24.04. I have not reproduced that crash or established its exact cause.
  • Windows ARM64: the wheel job builds successfully and reports 154 passed, 8 skipped, and 11,288 subtests passed, including all complex tests. Its sole failure is test_dap opening the dated remote dataset. An independent request now returns HTTP 404 for that dataset's DDS endpoint, while the next day's endpoint returns HTTP 200. This supports a remote-data availability issue, though it does not establish why the file was unavailable during the original run.

The new workflows are waiting for approval again, including Build and Test and Wheels. Could you approve them when convenient?

@ZedThree ZedThree left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This took me a minute to understand, but I think I understand now. We can use dtype_is_complex(self.dtype) in these places because we have already checked if auto_complex == True by then. So then this call can only be true if we have an actual complex dtype and we're using auto_complex.

Thanks @FFspiky!

This branch has not been deployed

No deployments
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.

Writing a file with auto_complex=True fails when using a dimension for the complex components

3 participants