From 783953b37b44cd9c53ce5970ee6403168c82fa36 Mon Sep 17 00:00:00 2001 From: Aditya Singh Date: Wed, 5 Aug 2026 07:52:14 -0700 Subject: [PATCH] Fix CircularRegionOfInterest.pixels_in_region omitting boundary pixels pixels_in_region enumerated range(floor(c - r), ceil(c + r)). range excludes its stop value, so pixels at the +x and +y extremes of the disc were never passed to is_inside at all. is_inside tests a closed disc with <=, so it accepts those pixels, leaving the region asymmetric about its own centre. Add 1 to both upper bounds so every pixel is_inside accepts is enumerated. The existing radius=1 expectation in test_regionofinterest.py contradicted the radius=1.01 expectation directly below it. Both enclose the same pixel centres under a closed disc, so the radius=1 case is corrected here. RectangularRegionOfInterest and PolygonRegionOfInterest were swept for the same off-by-one and are unaffected: the rectangle's is_inside uses a half-open interval that matches its range bounds, and the polygon's ray caster excludes its top and right edges to match. Fixes #1889 --- neo/core/regionofinterest.py | 8 +++-- neo/test/coretest/test_regionofinterest.py | 39 +++++++++++++++++++++- 2 files changed, 44 insertions(+), 3 deletions(-) diff --git a/neo/core/regionofinterest.py b/neo/core/regionofinterest.py index f797089d5..50e8199ba 100644 --- a/neo/core/regionofinterest.py +++ b/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]) diff --git a/neo/test/coretest/test_regionofinterest.py b/neo/test/coretest/test_regionofinterest.py index eb59c6dff..34b467bde 100644 --- a/neo/test/coretest/test_regionofinterest.py +++ b/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):