Fix CircularRegionOfInterest.pixels_in_region omitting boundary pixels

neo/core/regionofinterest.py

@@ -79,9 +79,13 @@ def is_inside(self, x, y):
 
     def pixels_in_region(self):
         """Returns a list of pixels whose *centres* are within the circle"""
+        # `is_inside` tests a closed disc, so a pixel centre exactly `radius` away is
+        # inside. `range` excludes its stop value, so the upper bounds need a `+ 1` for
+        # those extremes to be offered to `is_inside` at all. Without it the region is
+        # not symmetric about its own centre.
         pixel_in_list = []
-        for y in range(int(floor(self.y - self.radius)), int(ceil(self.y + self.radius))):
-            for x in range(int(floor(self.x - self.radius)), int(ceil(self.x + self.radius))):
+        for y in range(int(floor(self.y - self.radius)), int(ceil(self.y + self.radius)) + 1):
+            for x in range(int(floor(self.x - self.radius)), int(ceil(self.x + self.radius)) + 1):
                 if self.is_inside(x, y):
                     pixel_in_list.append([x, y])
 

neo/test/coretest/test_regionofinterest.py

@@ -8,11 +8,48 @@ class Test_CircularRegionOfInterest(unittest.TestCase):
 
     def test_result(self):
         seq = ImageSequence([[[]]], spatial_scale=1, frame_duration=20 * pq.ms)
-        self.assertEqual((CircularRegionOfInterest(seq, 6, 6, 1).pixels_in_region()), [[6, 5], [5, 6], [6, 6]])
+        # `is_inside` tests a closed disc, so radius 1 and radius 1.01 enclose the same
+        # pixel centres. The radius 1 expectation used to be [[6, 5], [5, 6], [6, 6]],
+        # which contradicted both the line below it and `is_inside` itself.
+        self.assertEqual(
+            (CircularRegionOfInterest(seq, 6, 6, 1).pixels_in_region()), [[6, 5], [5, 6], [6, 6], [7, 6], [6, 7]]
+        )
         self.assertEqual(
             (CircularRegionOfInterest(seq, 6, 6, 1.01).pixels_in_region()), [[6, 5], [5, 6], [6, 6], [7, 6], [6, 7]]
         )
 
+    def test_pixels_in_region_matches_is_inside(self):
+        """Every pixel `is_inside` accepts must be enumerated by `pixels_in_region`."""
+        seq = ImageSequence([[[]]], spatial_scale=1, frame_duration=20 * pq.ms)
+        for x, y, radius in ((6, 6, 1), (10, 10, 5), (6, 6, 2.5), (7, 4, 3), (5, 5, 1.01)):
+            roi = CircularRegionOfInterest(seq, x, y, radius)
+            enumerated = {tuple(pixel) for pixel in roi.pixels_in_region()}
+            margin = int(radius) + 2
+            accepted = {
+                (px, py)
+                for py in range(y - margin, y + margin + 1)
+                for px in range(x - margin, x + margin + 1)
+                if roi.is_inside(px, py)
+            }
+            self.assertEqual(enumerated, accepted, f"disagreement for x={x}, y={y}, radius={radius}")
+
+    def test_pixels_in_region_is_symmetric_about_centre(self):
+        """The disc must reach as far right and up as it does left and down."""
+        seq = ImageSequence([[[]]], spatial_scale=1, frame_duration=20 * pq.ms)
+        roi = CircularRegionOfInterest(seq, 10, 10, 5)
+        pixels = roi.pixels_in_region()
+
+        # (15, 10) sits exactly `radius` from the centre, so the closed disc includes it.
+        self.assertTrue(roi.is_inside(15, 10))
+        self.assertIn([15, 10], pixels)
+        self.assertIn([10, 15], pixels)
+
+        xs = [pixel[0] for pixel in pixels]
+        ys = [pixel[1] for pixel in pixels]
+        self.assertEqual((min(xs), max(xs)), (5, 15))
+        self.assertEqual((min(ys), max(ys)), (5, 15))
+        self.assertEqual(len(pixels), 81)
+
 
 class Test_RectangularRegionOfInterest(unittest.TestCase):