From 153045a3e4e2eefb15f88176d14f3bbc1a40fd1d Mon Sep 17 00:00:00 2001
From: Ross <xkjq@ymail.com>
Date: Mon, 30 Jun 2025 11:04:39 +0100
Subject: [PATCH] optimise some dicom manipulations

---
 atlas/api.py      | 44 ++++++++++++++++++++-------------------
 generic/models.py | 52 +++++++++++++++++++++--------------------------
 2 files changed, 46 insertions(+), 50 deletions(-)

diff --git a/atlas/api.py b/atlas/api.py
index 79501741..5dec2945 100644
--- a/atlas/api.py
+++ b/atlas/api.py
@@ -357,36 +357,38 @@ def series_split_by_tag(request, series_id: int, dicom_tag: str):
 
     image_map = defaultdict(list)
 
-    # Split teh associated images
-    for image in series.images.all():
+    # Use iterator to avoid loading all images into memory
+    for image in series.images.iterator():
         ds = image.get_dicom_data()
-
         if dicom_tag in ds:
             val = ds[dicom_tag].value
-
-            if type(val) == pydicom.multival.MultiValue:
-                image_map[",".join((val))].append(image)
+            if isinstance(val, pydicom.multival.MultiValue):
+                key = ",".join(map(str, val))
             else:
-                image_map[val].append(image)
+                key = str(val)
+            image_map[key].append(image)
         else:
             return [f"{dicom_tag} not found {image}"]
 
+    case = list(series.case.all())
+    authors = list(series.author.all())
     new_series = []
-    case = series.case.all()
-    authors = series.author.all()
 
-    for n, option in enumerate(image_map):
-        if n == 0:
-            series.images.set(image_map[option])
-
-        else:
-            series.pk = None
-            series.save()
-            series.case.set(case)
-            series.author.set(authors)
-            series.images.set(image_map[option])
-
-        new_series.append(series.pk)
+    # Use a transaction for all DB changes
+    with transaction.atomic():
+        for n, (option, images) in enumerate(image_map.items()):
+            if n == 0:
+                # Update the original series
+                series.images.set(images)
+                new_series.append(series.pk)
+            else:
+                # Clone the series
+                series.pk = None
+                series.save()
+                series.case.set(case)
+                series.author.set(authors)
+                series.images.set(images)
+                new_series.append(series.pk)
 
     return new_series
 
diff --git a/generic/models.py b/generic/models.py
index c7ff7ca9..2e2351a4 100644
--- a/generic/models.py
+++ b/generic/models.py
@@ -568,41 +568,35 @@ class SeriesBase(models.Model):
             n = n + 1
 
     def order_by_dicom(self, field="SliceLocation"):
+        """
+        Orders images in the series by the given DICOM field (default: SliceLocation).
+        Only keeps minimal data in memory and uses bulk_update for efficiency.
+        """
         images = self.images.filter(removed=False)
+        sortable = []
 
+        for img in images:
+            try:
+                ds = pydicom.dcmread(img.image.path, stop_before_pixels=True)
+                value = getattr(ds, field, None)
+                if value is not None:
+                    sortable.append((value, img))
+            except Exception:
+                continue  # Skip unreadable or invalid DICOMs
 
-        files = []
-
-        for i in images:
-            files.append((i, pydicom.dcmread(i.image.path)))
-
-        # print("file count: {}".format(len(files)))
-
-        # skip files with no SliceLocation (eg scout views)
-        slices = []
-        map = {}
-        skipcount = 0
-        for i, f in files:
-            if hasattr(f, field):
-                slices.append(f)
-                # map[f.SliceLocation] = i
-                map[f[field].value] = i
-            else:
-                skipcount = skipcount + 1
-
-        if not slices:
+        if not sortable:
             return
 
-        # ensure they are in the correct order
-        slices = sorted(slices, key=lambda s: s[field].value)
+        # Sort by the DICOM field value
+        sortable.sort(key=lambda x: x[0])
 
-        # print(slices)
-        n = 1
-        for f in slices:
-            i = map[f[field].value]
-            i.position = n
-            i.save()
-            n = n + 1
+        # Assign new positions and collect for bulk update
+        for idx, (value, img) in enumerate(sortable, start=1):
+            img.position = idx
+
+        # Bulk update all changed images
+        SeriesImage = self.images.model
+        SeriesImage.objects.bulk_update([img for _, img in sortable], ['position'])
 
     def get_total_image_size(self, refresh=False):
         """Returns the total size of all images in the series.