Skip to content

Commit 368c145

Browse files
committed
ENH: Avoid duplicating objects, note that coordinate family mappings are shared after with_name()
1 parent 107ead6 commit 368c145

File tree

2 files changed

+23
-3
lines changed

2 files changed

+23
-3
lines changed

nibabel/pointset.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,12 @@ def get_names(self):
212212
return list(self._coords)
213213

214214
def with_name(self, name: str) -> Self:
215-
new = replace(self, coordinates=self._coords[name])
215+
new_coords = self._coords[name]
216+
if new_coords is self.coordinates:
217+
return self
218+
# Make a copy, preserving all dataclass fields
219+
new = replace(self, coordinates=new_coords)
220+
# Conserve exact _coords mapping
216221
new._coords = self._coords
217222
return new
218223

nibabel/tests/test_pointset.py

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -250,13 +250,28 @@ def test_names(self):
250250
assert np.allclose(cfm.with_name('original').coordinates, coords)
251251

252252
cfm.add_coordinates('shifted', coords + 1)
253-
assert cfm.get_names() == ['original', 'shifted']
253+
assert set(cfm.get_names()) == {'original', 'shifted'}
254254
shifted = cfm.with_name('shifted')
255255
assert np.allclose(shifted.coordinates, coords + 1)
256-
assert shifted.get_names() == ['original', 'shifted']
256+
assert set(shifted.get_names()) == {'original', 'shifted'}
257257
original = shifted.with_name('original')
258258
assert np.allclose(original.coordinates, coords)
259259

260+
# Avoid duplicating objects
261+
assert original.with_name('original') is original
262+
# But don't try too hard
263+
assert original.with_name('original') is not cfm
264+
265+
# with_name() preserves the exact coordinate mapping of the source object.
266+
# Modifications of one are immediately available to all others.
267+
# This is currently an implementation detail, and the expectation is that
268+
# a family will be created once and then queried, but this behavior could
269+
# potentially become confusing or relied upon.
270+
# Change with care.
271+
shifted.add_coordinates('shifted-again', coords + 2)
272+
shift2 = shifted.with_name('shifted-again')
273+
shift3 = cfm.with_name('shifted-again')
274+
260275

261276
class H5ArrayProxy:
262277
def __init__(self, file_like, dataset_name):

0 commit comments

Comments
 (0)