Add check in PlaneSegmentation init for required cols (#2102)

Author: rly

CHANGELOG.md

@@ -4,7 +4,8 @@
 
 ### Bug fixes
 - Fixed reading and exporting of files written with NWB Schema < 2.9.0 that contained a reference to the electrodes table. @rly [#2112](https://github.com/NeurodataWithoutBorders/pynwb/pull/2112)
-- Skip streaming tests gracefully if offline. @rly [#2113](https://github.com/NeurodataWithoutBorders/pynwb/pull/2113)
+- Updated tests to skip streaming tests gracefully if offline. @rly [#2113](https://github.com/NeurodataWithoutBorders/pynwb/pull/2113)
+- Added check in `PlaneSegmentation` constructor for required columns. @rly [#2102](https://github.com/NeurodataWithoutBorders/pynwb/pull/2102)
 
 
 ## PyNWB 3.1.0 (July 8, 2025)

src/pynwb/ophys.py

@@ -354,12 +354,24 @@ def __init__(self, **kwargs):
         imaging_plane, reference_images = popargs('imaging_plane', 'reference_images', kwargs)
         if kwargs['name'] is None:
             kwargs['name'] = imaging_plane.name
+
+        if kwargs["columns"]:
+            # check for required ROI columns if table is initialized with non-empty columns 
+            if any(len(c) > 0 for c in kwargs["columns"]):
+                for c in kwargs["columns"]:
+                    if c.name in ("image_mask", "pixel_mask", "voxel_mask"):
+                        break
+                else:
+                    raise ValueError("Must provide at least one of 'image_mask', 'pixel_mask', or 'voxel_mask' columns")
+        elif kwargs["id"]:  # there are also no columns
+            raise ValueError("Must provide at least one of 'image_mask', 'pixel_mask', or 'voxel_mask' columns")
+
         super().__init__(**kwargs)
         self.imaging_plane = imaging_plane
         if isinstance(reference_images, ImageSeries):
             reference_images = (reference_images,)
         self.reference_images = reference_images
-
+        
     @docval({'name': 'pixel_mask', 'type': 'array_data', 'default': None,
              'doc': 'pixel mask for 2D ROIs: [(x1, y1, weight1), (x2, y2, weight2), ...]',
              'shape': (None, 3)},

tests/unit/test_ophys.py

@@ -1,6 +1,7 @@
 import warnings
 
 import numpy as np
+from hdmf.common import VectorData
 
 from pynwb.base import TimeSeries, ProcessingModule
 from pynwb.device import Device
@@ -142,7 +143,7 @@ def test_manifold_deprecated(self):
                 indicator='indicator',
                 location='location',
                 manifold=(1, 1, (2, 2, 2)))
-        
+
         # create object with deprecated argument
         with self.assertRaisesWith(ValueError, msg):
             ImagingPlane(**kwargs)
@@ -227,7 +228,7 @@ def test_init_description_optional(self):
                     'indicator',
                     'location',
                 )
-    
+
     def test_init_missing_required_args(self):
         """Check that ImagingPlane raises an error if required args are missing."""
         oc, device = self.set_up_dependencies()
@@ -527,7 +528,7 @@ def test_init(self):
         self.assertEqual(iS.name, 'test_iS')
         self.assertEqual(iS.plane_segmentations[ps.name], ps)
         self.assertEqual(iS[ps.name], iS.plane_segmentations[ps.name])
-    
+
     def test_add_segementation(self):
         ps = create_plane_segmentation()
         iS = ImageSegmentation(name='test_iS')
@@ -594,6 +595,45 @@ def test_init_no_name(self):
         )
         self.assertEqual(pS.name, ip.name)
 
+    def test_init_missing_roi_col_with_ids(self):
+        """If no roi column is provided and ids were provided, an error should be raised"""
+        iSS, ip = self.set_up_dependencies()
+        msg = "Must provide at least one of 'image_mask', 'pixel_mask', or 'voxel_mask' columns"
+        with self.assertRaises(ValueError, msg=msg):
+            PlaneSegmentation(
+                description='description',
+                imaging_plane=ip,
+                name='test_name',
+                reference_images=iSS,
+                id=[1, 2, 3],
+            )
+
+    def test_init_missing_roi_col_with_columns(self):
+        """If no roi column is provided and other non-empty columns are provided, an error should be raised"""
+        iSS, ip = self.set_up_dependencies()
+        msg = "Must provide at least one of 'image_mask', 'pixel_mask', or 'voxel_mask' columns"
+        with self.assertRaises(ValueError, msg=msg):
+            PlaneSegmentation(
+                description='description',
+                imaging_plane=ip,
+                name='test_name',
+                reference_images=iSS,
+                columns=[VectorData(name="custom_col", description="custom col", data=[1, 2, 3])],
+                id=[1, 2, 3],
+            )
+
+    def test_init_missing_roi_col_with_empty_columns(self):
+        """If no roi column is provided and other columns are empty, no error should be raised"""
+        iSS, ip = self.set_up_dependencies()
+        pS = PlaneSegmentation(
+            description='description',
+            imaging_plane=ip,
+            name='test_name',
+            reference_images=iSS,
+            columns=[VectorData(name="custom_col", description="custom col")],
+        )
+        self.assertEqual(len(pS), 0)
+
     def test_add_pixel_mask(self):
         pix_mask = [[1, 2, 1.0], [3, 4, 1.0], [5, 6, 1.0],
                     [7, 8, 2.0], [9, 10, 2.0]]
@@ -659,7 +699,6 @@ def test_add_roi_missing_params(self):
         with self.assertRaises(ValueError, msg=msg):
             pS.add_roi()
 
-
     def test_conversion_of_2d_pixel_mask_to_image_mask(self):
         pixel_mask = [[0, 0, 1.0], [1, 0, 2.0], [2, 0, 2.0]]