BF: Return complex data from get_fdata() for complex images - #1528
Open
Leonard013 wants to merge 1 commit into
Open
BF: Return complex data from get_fdata() for complex images#1528Leonard013 wants to merge 1 commit into
Leonard013 wants to merge 1 commit into
Conversation
``DataobjImage.get_fdata()`` defaulted to ``dtype=np.float64``, so calling it on a complex-valued image silently discarded the imaginary part (with a NumPy ComplexWarning). Make the default dtype depend on the image: when no ``dtype`` is passed and the data object has a complex dtype, return ``np.promote_types(obj_dtype, np.complex128)`` -- i.e. at least ``complex128`` (preserving the imaginary part), but wider complex types such as ``complex256`` are preserved rather than silently downcast. Otherwise return ``np.float64`` as before. An explicitly passed ``dtype`` is always respected, so real-valued images and callers that request a specific dtype are unaffected. Closes nipygh-975. This change was developed with AI assistance (Claude Code); it has been reviewed and verified locally by the commit author. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
Closes #975.
Why
get_fdata()defaults todtype=np.float64. For a complex-valued image thissilently discards the imaginary part (emitting a NumPy
ComplexWarning), sousers of complex data (e.g. MR spectroscopy via spec2nii) lose data unless they
know to pass
dtype=np.complex64/128explicitly.In #975 both maintainers agreed on the fix. The last substantive comment
(@matthew-brett, 2020-12-05) settles the scope:
@effigies (2020-12-03): "I think detecting the
complexcase is worth doing."(He also floated a broader dtype-resolution overhaul with a new
modeargument;this PR implements only the minimal, agreed behavior and does not preclude that
larger work.)
What
In
DataobjImage.get_fdata():dtype=np.float64todtype=None(sentinelfor "not specified").
dtype is None: ifself._dataobj.dtypeis complex, resolve tonp.promote_types(obj_dtype, np.complex128)(floors atcomplex128butpreserves wider complex types, e.g.
complex256); otherwisenp.float64(unchanged).
dtypeis passed: respect it exactly (unchanged)._fdata_cachetype hints fromnp.floatingtonp.inexact, the docstring, and broaden the validation error message to"floating point or complex type".
Real-valued images and every explicit-
dtypecall are unaffected. Caching isunchanged (dtype is resolved to a concrete dtype before the cache lookup).
Design note — wider-than-complex128 data (complex256)
NIfTI datatype code 2048 maps to
np.clongdouble, which on Linux/x86 (theprimary CI platform) is
complex256— wider thancomplex128. A naivedtype = np.complex128default would silently downcast such data (the same classof silent loss this PR fixes). Using
np.promote_types(obj_dtype, np.complex128)returns
complex128forcomplex64/complex128andcomplex256forcomplex256— never narrower, never lossy.Tests
Added to
nibabel/tests/test_image_api.py:test_get_fdata_complex[complex64/complex128]— array and proxy images:default
get_fdata()returnscomplex128and preserves the imaginary part;explicit
dtype=complex64/128respected; explicitdtype=float64still dropsimag.
test_get_fdata_real_unchanged[uint8/int16/float32/float64]— real imagesstill default to
float64(no regression).test_get_fdata_complex256—clongdoubleimage preservescomplex256,guarded with
skipif(np.dtype(np.clongdouble) == np.dtype(np.complex128))soit runs on Linux/x86 CI and no-ops where
clongdoubleisn't wider.Fail-before / pass-after verified. Full suite adds no regressions (the complex
cases fail on
masterwith the imaginary part dropped, pass here).This changes the default return dtype for complex images (
float64→complex128). Code that callsget_fdata()on a complex image and assumes areal
float64array would now receive complex output. This is the intended fix(the old behavior was lossy and warned), but it is a behavior change — happy to
add an "API changes" changelog note, or a deprecation cycle if you'd prefer.
Callers wanting the old behavior can pass
dtype=np.float64.Since #975 is from 2020, flagging for @effigies / @matthew-brett: is the minimal
complex128-default approach (vs. the broader dtype-modeidea) still what you'dlike? Glad to adjust.
(Changelog fragment omitted from the commit — nibabel entries reference the PR
number and there's no open "upcoming" section at HEAD; happy to add one.)