Skip to content

Commit

Permalink
FIX: Restore access to private attr Nifti1Extension._content
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
effigies committed Oct 15, 2024
1 parent 1712cb0 commit e97f572
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 7 deletions.
18 changes: 11 additions & 7 deletions nibabel/nifti1.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__(
Expand All @@ -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.
Expand Down Expand Up @@ -394,15 +398,15 @@ 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:
code = extension_codes.label[self.code]
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 (
Expand All @@ -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:
Expand All @@ -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
Expand Down Expand Up @@ -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()
Expand Down
27 changes: 27 additions & 0 deletions nibabel/tests/test_nifti1.py
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down

0 comments on commit e97f572

Please sign in to comment.