CircularRegionOfInterest.pixels_in_region omits boundary pixels that its own is_inside accepts

[Arthur031221]

Describe the bug

CircularRegionOfInterest.is_inside() accepts pixels that pixels_in_region()
never returns, so the region is not symmetric about its own centre. It looks
like a regression from b43026d rather than a deliberate choice, and the
example in examples/plot_imageseq.py is affected.

is_inside() tests a closed disc (<=), so a pixel exactly radius away is
inside. pixels_in_region() scans range(int(floor(c - r)), int(ceil(c + r))),
and range stops one short of its endpoint, so pixels at the +x and +y
extremes are never passed to is_inside() at all. They aren't rejected, they
are never asked about.

This only bites when a pixel centre lands at exactly distance radius, which
for a whole-number centre means a whole-number radius. radius=2.5 and
radius=1.01 are unaffected. Whole numbers are the common case though, and
both ROIs in examples/plot_imageseq.py use them.

To Reproduce

import numpy as np
import quantities as pq
from neo.core import ImageSequence
from neo.core.regionofinterest import CircularRegionOfInterest

seq = ImageSequence(np.zeros((2, 120, 120)), sampling_rate=1 * pq.Hz,
                    spatial_scale=1 * pq.micrometer, units="V")

roi = CircularRegionOfInterest(seq, 10, 10, 5)      # from examples/plot_imageseq.py
pixels = roi.pixels_in_region()

print(roi.is_inside(15, 10), [15, 10] in pixels)    # True False
print(roi.is_inside(5, 10), [5, 10] in pixels)      # True True

xs = [p[0] for p in pixels]
print(min(xs), max(xs))                             # 5 14, for a circle centred on 10

accepted = sum(1 for y in range(120) for x in range(120) if roi.is_inside(x, y))
print(len(pixels), accepted)                        # 79 81

(15, 10) sits exactly 5 pixels from the centre. is_inside() says it is in the
region. It is never enumerated. The same holds for (10, 15). The mirror pixels
(5, 10) and (10, 5) are both returned, so the region reaches 5 pixels to the
left of the centre and 4 to the right.

The other ROI in the same example, CircularRegionOfInterest(seq, 50, 50, 25),
returns 1959 pixels where is_inside() accepts 1961.

Expected behaviour

pixels_in_region() should return every pixel is_inside() accepts. For
radius=5 centred on 10 that is 81 pixels spanning x 5 to 15, symmetric about
the centre.

RectangularRegionOfInterest already behaves this way. Its is_inside() uses a
half-open interval, which matches what its own range() bounds enumerate, and
sweeping 1056 centre/width/height combinations turns up no disagreement between
its predicate and its enumeration.

Environment:

• OS: Linux 6.17, Ubuntu
• Python version: 3.12.3
• Neo version: 0.15.dev0 at 35cbce7, and 0.14.5 (nothing has touched
  regionofinterest.py since that tag, so a pip install reproduces it)
• NumPy version: 2.5.1

Additional context

The history suggests this was collateral damage rather than a decision, and
there is one thing only you can tell me.

Before b43026d the loop read:

for y in range(self.y - self.radius, self.y + self.radius + 1):

Inclusive, matching the <= that is_inside() has carried since the file was
added. b43026d ("Make RegionOfInterest work with float arguments",
2019-09-26 10:55) rewrote the bounds with floor/ceil and the +1 went with
it. Twenty-nine minutes later 52977d3 ("fix tests; pep8") changed the
radius-1 assertion in test_regionofinterest.py to the three-pixel list and
added a radius-1.01 case asserting the five-pixel plus shape.

So: was that assertion change a deliberate call that a pixel exactly on the
circle is outside it, or was it getting the suite green after the bounds
changed? If it was deliberate then is_inside() is what needs fixing, not the
enumeration, and the fix is the opposite of what I would have written.

PolygonRegionOfInterest has the same bounding box under-scan. A square with
vertices at (10,10), (20,10), (20,20), (10,20) enumerates x 10 to 19 and y 10
to 19. I haven't investigated whether that one matters, since polygon has no
is_inside() to disagree with.

I have a fix and regression tests ready, using
range(int(ceil(c - r)), int(floor(c + r)) + 1) so the scan covers the closed
interval exactly rather than over-scanning past it. Across 44 centre and radius
combinations, including half-integer centres and fractional radii, it returns
exactly the set is_inside() accepts, with nothing missing and nothing extra.

Against 35cbce7, with those two lines changed, the radius-1 assertion restored
to the five-pixel list and two regression tests added to
Test_CircularRegionOfInterest, neo/test/coretest runs 621 passed, 12
skipped, 0 failed, up from 619 passed, 12 skipped, 0 failed. Reverting the two
lines and leaving everything else gives 3 failures. Nothing else in the suite
moves either way.

Happy to open a PR, once you tell me which side of it you want changed.