From 368c145543dd7d0742148fea55eeafa32dd69640 Mon Sep 17 00:00:00 2001
From: Chris Markiewicz <effigies@gmail.com>
Date: Fri, 22 Sep 2023 09:50:29 -0400
Subject: [PATCH] ENH: Avoid duplicating objects, note that coordinate family
 mappings are shared after with_name()

---
 nibabel/pointset.py            |  7 ++++++-
 nibabel/tests/test_pointset.py | 19 +++++++++++++++++--
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/nibabel/pointset.py b/nibabel/pointset.py
index fed5513b4..db70f815d 100644
--- a/nibabel/pointset.py
+++ b/nibabel/pointset.py
@@ -212,7 +212,12 @@ def get_names(self):
         return list(self._coords)
 
     def with_name(self, name: str) -> Self:
-        new = replace(self, coordinates=self._coords[name])
+        new_coords = self._coords[name]
+        if new_coords is self.coordinates:
+            return self
+        # Make a copy, preserving all dataclass fields
+        new = replace(self, coordinates=new_coords)
+        # Conserve exact _coords mapping
         new._coords = self._coords
         return new
 
diff --git a/nibabel/tests/test_pointset.py b/nibabel/tests/test_pointset.py
index 2a0b54561..31c5335a1 100644
--- a/nibabel/tests/test_pointset.py
+++ b/nibabel/tests/test_pointset.py
@@ -250,13 +250,28 @@ def test_names(self):
         assert np.allclose(cfm.with_name('original').coordinates, coords)
 
         cfm.add_coordinates('shifted', coords + 1)
-        assert cfm.get_names() == ['original', 'shifted']
+        assert set(cfm.get_names()) == {'original', 'shifted'}
         shifted = cfm.with_name('shifted')
         assert np.allclose(shifted.coordinates, coords + 1)
-        assert shifted.get_names() == ['original', 'shifted']
+        assert set(shifted.get_names()) == {'original', 'shifted'}
         original = shifted.with_name('original')
         assert np.allclose(original.coordinates, coords)
 
+        # Avoid duplicating objects
+        assert original.with_name('original') is original
+        # But don't try too hard
+        assert original.with_name('original') is not cfm
+
+        # with_name() preserves the exact coordinate mapping of the source object.
+        # Modifications of one are immediately available to all others.
+        # This is currently an implementation detail, and the expectation is that
+        # a family will be created once and then queried, but this behavior could
+        # potentially become confusing or relied upon.
+        # Change with care.
+        shifted.add_coordinates('shifted-again', coords + 2)
+        shift2 = shifted.with_name('shifted-again')
+        shift3 = cfm.with_name('shifted-again')
+
 
 class H5ArrayProxy:
     def __init__(self, file_like, dataset_name):