From ef6df4e7ec5b5df9721dd2cf293de5e478fa8009 Mon Sep 17 00:00:00 2001 From: Chris Markiewicz Date: Tue, 15 Oct 2024 14:04:51 -0400 Subject: [PATCH] FIX: Restore access to private attr Nifti1Extension._content gh-1336 reused the private attribute ``ext._content`` to exclusively refer to the ``bytes`` representation of the extension contents. This neglected that subclasses might depend on this implementation detail. Let's be nice to people and rename the attribute to ``_raw`` and provide a ``_content`` property that calls ``self.get_content()``. Also adds a test to ensure that multiple accesses continue to work as expected. --- nibabel/nifti1.py | 18 +++++++++++------- nibabel/tests/test_nifti1.py | 27 +++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 7 deletions(-) diff --git a/nibabel/nifti1.py b/nibabel/nifti1.py index f0bd91fc4..37f75db10 100644 --- a/nibabel/nifti1.py +++ b/nibabel/nifti1.py @@ -326,7 +326,7 @@ class NiftiExtension(ty.Generic[T]): code: int encoding: str | None = None - _content: bytes + _raw: bytes _object: T | None = None def __init__( @@ -351,10 +351,14 @@ def __init__( self.code = extension_codes.code[code] # type: ignore[assignment] except KeyError: self.code = code # type: ignore[assignment] - self._content = content + self._raw = content if object is not None: self._object = object + @property + def _content(self): + return self.get_object() + @classmethod def from_bytes(cls, content: bytes) -> Self: """Create an extension from raw bytes. @@ -394,7 +398,7 @@ def _sync(self) -> None: and updates the bytes representation accordingly. """ if self._object is not None: - self._content = self._mangle(self._object) + self._raw = self._mangle(self._object) def __repr__(self) -> str: try: @@ -402,7 +406,7 @@ def __repr__(self) -> str: except KeyError: # deal with unknown codes code = self.code - return f'{self.__class__.__name__}({code}, {self._content!r})' + return f'{self.__class__.__name__}({code}, {self._raw!r})' def __eq__(self, other: object) -> bool: return ( @@ -425,7 +429,7 @@ def get_code(self): def content(self) -> bytes: """Return the extension content as raw bytes.""" self._sync() - return self._content + return self._raw @property def text(self) -> str: @@ -452,7 +456,7 @@ def get_object(self) -> T: instead. """ if self._object is None: - self._object = self._unmangle(self._content) + self._object = self._unmangle(self._raw) return self._object # Backwards compatibility @@ -488,7 +492,7 @@ def write_to(self, fileobj: ty.BinaryIO, byteswap: bool = False) -> None: extinfo = extinfo.byteswap() fileobj.write(extinfo.tobytes()) # followed by the actual extension content, synced above - fileobj.write(self._content) + fileobj.write(self._raw) # be nice and zero out remaining part of the extension till the # next 16 byte border pad = extstart + rawsize - fileobj.tell() diff --git a/nibabel/tests/test_nifti1.py b/nibabel/tests/test_nifti1.py index f0029681b..053cad755 100644 --- a/nibabel/tests/test_nifti1.py +++ b/nibabel/tests/test_nifti1.py @@ -1250,6 +1250,33 @@ def test_extension_content_access(): assert json_ext.json() == {'a': 1} +def test_legacy_underscore_content(): + """Verify that subclasses that depended on access to ._content continue to work.""" + import io + import json + + class MyLegacyExtension(Nifti1Extension): + def _mangle(self, value): + return json.dumps(value).encode() + + def _unmangle(self, value): + if isinstance(value, bytes): + value = value.decode() + return json.loads(value) + + ext = MyLegacyExtension(0, '{}') + + assert isinstance(ext._content, dict) + # Object identity is not broken by multiple accesses + assert ext._content is ext._content + + ext._content['val'] = 1 + + fobj = io.BytesIO() + ext.write_to(fobj) + assert fobj.getvalue() == b'\x20\x00\x00\x00\x00\x00\x00\x00{"val": 1}' + bytes(14) + + def test_extension_codes(): for k in extension_codes.keys(): Nifti1Extension(k, 'somevalue')