BF: Nibabel header extension access

[wtclarke]

Fixes header extension access arising from nipy/nibabel#1336

@effigies You gave me plenty of warning about this, and I completely failed to test or do anything about it, but just to let you know that users reported errors like this:

File d:\Conda\envs\dan_fsl_mrs\Lib\site-packages\nifti_mrs\nifti_mrs.py:122, in NIFTI_MRS.__init__(self, validate_on_creation, *args, **kwargs)
    117     if 44 not in hdr_ext_codes:
    118         raise NotNIFTI_MRS('NIFTI-MRS must have a header extension.')
    120     self._hdr_ext = Hdr_Ext.from_header_ext(
    121         json.loads(
--> 122             self.header.extensions[hdr_ext_codes.index(44)].get_content()))
    124 # Some validation upon creation
    125 if validate_on_creation:

File d:\Conda\envs\dan_fsl_mrs\Lib\site-packages\nibabel\nifti1.py:455, in NiftiExtension.get_object(self)
    448 """Return the extension content in its runtime representation.
    449 
    450 This method may return a different type for each extension type.
    451 For simple use cases, consider using ``.content``, ``.text`` or ``.json()``
    452 instead.
    453 """
    454 if self._object is None:
--> 455     self._object = self._unmangle(self._content)
    456 return self._object

File d:\Conda\envs\dan_fsl_mrs\Lib\site-packages\nibabel\nifti1.py:388, in NiftiExtension._unmangle(self, content)
    387 def _unmangle(self, content: bytes) -> T:
--> 388     raise NotImplementedError

Is this what you intended?

[effigies]

I had actually tried not to break old code and just provide better access mechanisms for common use case and lay the groundwork for writing cleaner subclasses.

Does 5.3.1 fix things? It resolved issues for dcmstack.

[wtclarke]

5.3.1 didn't help, the code still hit the NotImplementedError in _unmangle.

[effigies]

Okay, there are some indirections I don't understand because of fslpy.Image. I suspect what's happening is that I gave a meaning to code 44, so when you're reading in the extension, you're not getting the old fallback of Nifti1Extension but instead the new NiftiExtension:

https://github.com/nipy/nibabel/blob/e9ed337c0e549acc9b9e1b2eee809cfb154b9450/nibabel/nifti1.py#L674

In this case, get_content() should now present a cached dictionary, parsed from JSON, which could be modified and updated. If you want read-only, .json (attribute, not method) should work.

One thing we could do is revert it to a Nifti1Extension subclass where get_content() continues to return bytes, but warn that it will change to a dictionary in the future, and cut a bug-fix release. Let me know if you'd rather do that than adapt to the new API immediately.

[wtclarke]

I don't think this is the behaviour I'm seeing. E.g. if I run

import nibabel as nib
obj = nib.load('test_data/metab.nii.gz')
print(obj.header.extensions)
print(obj.header.extensions[0])
print(type(obj.header.extensions[0].content))
print(type(obj.header.extensions[0].text))
print(type(obj.header.extensions[0].json()))
obj.header.extensions[0].get_content()

I get

Nifti1Extensions(NiftiExtension(mrs, b'{"SpectrometerFrequency": [297.219948]...
NiftiExtension(mrs, b'{"SpectrometerFrequency": [297.219948], ...
<class 'bytes'>
<class 'str'>
<class 'dict'>

(I've truncate the first two outputs)
but then the final call gives

---------------------------------------------------------------------------
NotImplementedError                       Traceback (most recent call last)
Cell In[23], [line 8](vscode-notebook-cell:?execution_count=23&line=8)
      [6](vscode-notebook-cell:?execution_count=23&line=6) print(type(obj.header.extensions[0].text))
      [7](vscode-notebook-cell:?execution_count=23&line=7) print(type(obj.header.extensions[0].json()))
----> [8](vscode-notebook-cell:?execution_count=23&line=8) obj.header.extensions[0].get_content()

File ~/miniforge3/envs/nibabel_test_531/lib/python3.12/site-packages/nibabel/nifti1.py:459, in NiftiExtension.get_object(self)
    [452](https://file+.vscode-resource.vscode-cdn.net/Users/wclarke/Documents/Python/nifti_mrs_tools/tests/~/miniforge3/envs/nibabel_test_531/lib/python3.12/site-packages/nibabel/nifti1.py:452) """Return the extension content in its runtime representation.
    [453](https://file+.vscode-resource.vscode-cdn.net/Users/wclarke/Documents/Python/nifti_mrs_tools/tests/~/miniforge3/envs/nibabel_test_531/lib/python3.12/site-packages/nibabel/nifti1.py:453) 
    [454](https://file+.vscode-resource.vscode-cdn.net/Users/wclarke/Documents/Python/nifti_mrs_tools/tests/~/miniforge3/envs/nibabel_test_531/lib/python3.12/site-packages/nibabel/nifti1.py:454) This method may return a different type for each extension type.
    [455](https://file+.vscode-resource.vscode-cdn.net/Users/wclarke/Documents/Python/nifti_mrs_tools/tests/~/miniforge3/envs/nibabel_test_531/lib/python3.12/site-packages/nibabel/nifti1.py:455) For simple use cases, consider using ``.content``, ``.text`` or ``.json()``
    [456](https://file+.vscode-resource.vscode-cdn.net/Users/wclarke/Documents/Python/nifti_mrs_tools/tests/~/miniforge3/envs/nibabel_test_531/lib/python3.12/site-packages/nibabel/nifti1.py:456) instead.
    [457](https://file+.vscode-resource.vscode-cdn.net/Users/wclarke/Documents/Python/nifti_mrs_tools/tests/~/miniforge3/envs/nibabel_test_531/lib/python3.12/site-packages/nibabel/nifti1.py:457) """
    [458](https://file+.vscode-resource.vscode-cdn.net/Users/wclarke/Documents/Python/nifti_mrs_tools/tests/~/miniforge3/envs/nibabel_test_531/lib/python3.12/site-packages/nibabel/nifti1.py:458) if self._object is None:
--> [459](https://file+.vscode-resource.vscode-cdn.net/Users/wclarke/Documents/Python/nifti_mrs_tools/tests/~/miniforge3/envs/nibabel_test_531/lib/python3.12/site-packages/nibabel/nifti1.py:459)     self._object = self._unmangle(self._raw)
    [460](https://file+.vscode-resource.vscode-cdn.net/Users/wclarke/Documents/Python/nifti_mrs_tools/tests/~/miniforge3/envs/nibabel_test_531/lib/python3.12/site-packages/nibabel/nifti1.py:460) return self._object

File ~/miniforge3/envs/nibabel_test_531/lib/python3.12/site-packages/nibabel/nifti1.py:392, in NiftiExtension._unmangle(self, content)
    [391](https://file+.vscode-resource.vscode-cdn.net/Users/wclarke/Documents/Python/nifti_mrs_tools/tests/~/miniforge3/envs/nibabel_test_531/lib/python3.12/site-packages/nibabel/nifti1.py:391) def _unmangle(self, content: bytes) -> T:
--> [392](https://file+.vscode-resource.vscode-cdn.net/Users/wclarke/Documents/Python/nifti_mrs_tools/tests/~/miniforge3/envs/nibabel_test_531/lib/python3.12/site-packages/nibabel/nifti1.py:392)     raise NotImplementedError

NotImplementedError:

[effigies]

Ah, you're right. That's definitely an incorrect signature, as it would need an implementation, not just a type parameter, e.g.:

class JSONExtension(NiftiExtension[dict[str, Any]]):
    def _unmangle(self, raw):
        return json.loads(raw.decode())

    def _mangle(self, obj):
        return json.dumps(obj).encode()

So .json() (you're right, I used a method, not a property) is the way to go, as written.