Merged in bugfix/RAM-3906_picketfence_from_multiple_double_crop (pull…

… request #443)

RAM-3906 fix double crop and too large of a crop

Approved-by: Randy Taylor

docs/source/changelog.rst

@@ -22,6 +22,12 @@ Plan Generator
 * :bdg-success:`Feature` Manual names can now be passed for Winston-Lutz beams.
 * :bdg-warning:`Fixed` The R2 prefabricated plan files have been fixed to have the same energy and dose rate for all beams of a given plan.
 
+Picket Fence
+^^^^^^^^^^^^
+
+* :bdg-warning:`Fixed` When loading a picket fence from multiple images (``.from_multiple_images``), the images would
+   be double-cropped due to the ``crop_mm`` keyword argument being applied twice. This has been fixed.
+
 Image Generator
 ^^^^^^^^^^^^^^^
 
@@ -53,6 +59,8 @@ Core
 ^^^^
 
 * :bdg-primary:`Refactor` Performing :meth:`~pylinac.core.image.BaseImage.crop` on an image now allows for a ``pixels`` input of 0. This allows for a no-op crop.
+* :bdg-warning:`Fixed` Performing :meth:`~pylinac.core.image.BaseImage.crop` with ``pixels`` value too large that would result in an empty array now raises
+  an error.
 
 v 3.26.0
 --------

pylinac/core/image.py

@@ -654,6 +654,10 @@ def crop(
             self.array = self.array[:, pixels:]
         if "right" in edges:
             self.array = self.array[:, :-pixels]
+        if self.array.size == 0:
+            raise ValueError(
+                "Too many pixels removed; array is empty. Pass a smaller crop value."
+            )
 
     def flipud(self) -> None:
         """Flip the image array upside down in-place. Wrapper for np.flipud()"""

pylinac/picketfence.py

@@ -374,20 +374,26 @@ def from_multiple_images(
         kwargs
             Passed to :func:`~pylinac.core.image.load_multiples` and to the PicketFence constructor.
         """
+        # pop crop here so it's not passed to the combined image; only to the constructor
+        # otherwise it'll double crop
+        crop_mm = kwargs.pop("crop_mm", 3)
         with io.BytesIO() as stream:
             img = image.load_multiples(
                 path_list,
                 stretch_each=stretch_each,
                 method=method,
                 loader=PFDicomImage,
+                crop_mm=0,
                 **kwargs,
             )
             img.save(stream)
             stream.seek(0)
             # there is a parameter name mismatch between the PFDicomImage and PicketFence constructors
             # Dicom uses "use_filenames" and PicketFence uses "use_filename" 😖
             use_filename = kwargs.pop("use_filenames", False)
-            return cls(stream, mlc=mlc, use_filename=use_filename, **kwargs)
+            return cls(
+                stream, mlc=mlc, use_filename=use_filename, crop_mm=crop_mm, **kwargs
+            )
 
     @classmethod
     def from_bb_setup(

tests_basic/core/test_image.py

@@ -361,6 +361,12 @@ def test_crop_must_be_positive(self):
         with self.assertRaises(ValueError):
             self.img.crop(crop)
 
+    def test_crop_too_large(self):
+        """Crop must be less than half the image size"""
+        crop = 1000
+        with self.assertRaises(ValueError):
+            self.img.crop(crop)
+
     def test_filter(self):
         # test integer filter size
         filter_size = 3

tests_basic/test_picketfence.py

@@ -9,6 +9,7 @@
 from unittest import TestCase, skip
 
 import matplotlib.pyplot as plt
+import numpy as np
 from scipy import ndimage
 
 from pylinac.core import image
@@ -383,6 +384,23 @@ def test_loading_no_keywords(self):
         pf.analyze()
         self.assertAlmostEqual(pf.percent_passing, 100, delta=1)
 
+    def test_no_double_crop(self):
+        # see RAM-3906
+        path1 = get_file_from_cloud_test_repo([TEST_DIR, "combo-jaw.dcm"])
+        path2 = get_file_from_cloud_test_repo([TEST_DIR, "combo-mlc.dcm"])
+        # physical size of the images
+        img_base = image.load_multiples([path1, path2])
+        base_size = img_base.shape
+        # load the PF w/o crop
+        pf = PicketFence.from_multiple_images([path1, path2], crop_mm=0)
+        self.assertEqual(pf.image.shape, base_size)
+
+        # load the PF w/ crop
+        # * 2 because both edges get cropped
+        pixel_diff = int(round(3 * img_base.dpmm)) * 2
+        pf = PicketFence.from_multiple_images([path1, path2], crop_mm=3)
+        np.allclose(np.asarray(pf.image.shape), np.asarray(base_size) - pixel_diff)
+
 
 class TestPlottingSaving(TestCase):
     @classmethod