Make ImagingPlane.description optional (#2051)

Co-authored-by: Steph Prince <[email protected]>

Author: rly

CHANGELOG.md

@@ -34,7 +34,8 @@
 - Fixed missing `__nwbfields__` and `_fieldsname` for `NWBData` and its subclasses. @rly [#2082](https://github.com/NeurodataWithoutBorders/pynwb/pull/2082)
 - Fixed caching of the type map when using HDMF 4.1.0. @rly [#2087](https://github.com/NeurodataWithoutBorders/pynwb/pull/2087)
 - Removed use of complex numbers in scratch tutorial because of incompatibilities with HDMF 4.1.0. @stephprince [#2090](https://github.com/NeurodataWithoutBorders/pynwb/pull/2090/)
-
+- Made `ImagingPlane.description` optional to conform with the NWB Schema. @rly [#2051](https://github.com/NeurodataWithoutBorders/pynwb/pull/2051)
+  
 ### Documentation and tutorial enhancements
 - Added NWB AI assistant to the home page of the documentation. @magland [#2076](https://github.com/NeurodataWithoutBorders/pynwb/pull/2076)
 

src/pynwb/ophys.py

@@ -68,11 +68,13 @@ class ImagingPlane(NWBContainer):
     @docval(*get_docval(NWBContainer.__init__, 'name'),  # required
             {'name': 'optical_channel', 'type': (list, OpticalChannel),  # required
              'doc': 'One of possibly many groups storing channel-specific data.'},
-            {'name': 'description', 'type': str, 'doc': 'Description of this ImagingPlane.'},  # required
-            {'name': 'device', 'type': Device, 'doc': 'the device that was used to record'},  # required
-            {'name': 'excitation_lambda', 'type': float, 'doc': 'Excitation wavelength in nm.'},  # required
-            {'name': 'indicator', 'type': str, 'doc': 'Calcium indicator'},  # required
-            {'name': 'location', 'type': str, 'doc': 'Location of image plane.'},  # required
+            {'name': 'description', 'type': str, 'doc': 'Description of this ImagingPlane.', 'default': None},
+            {'name': 'device', 'type': Device, 'doc': 'the device that was used to record', 
+             'default': None},  # required
+            {'name': 'excitation_lambda', 'type': float, 'doc': 'Excitation wavelength in nm.', 
+             'default': None},  # required
+            {'name': 'indicator', 'type': str, 'doc': 'Calcium indicator',  'default': None},  # required
+            {'name': 'location', 'type': str, 'doc': 'Location of image plane.', 'default': None},  # required
             {'name': 'imaging_rate', 'type': float,
              'doc': 'Rate images are acquired, in Hz. If the corresponding TimeSeries is present, the rate should be '
                     'stored there instead.', 'default': None},
@@ -128,6 +130,21 @@ def __init__(self, **kwargs):
 
         if not isinstance(args_to_set['optical_channel'], list):
             args_to_set['optical_channel'] = [args_to_set['optical_channel']]
+
+        # Note: device, excitation_lambda, indicator, and location are required arguments.
+        # Description was made to be optional in PyNWB 3.1.0, however to avoid breaking API changes the order of
+        # the arguments needs to be maintained even though the optional arguments came before the required ones.
+        # So in docval these required arguments are displayed as optional when really they are required. 
+        # This section can be removed when positional arguments are no longer allowed.
+        if args_to_set['device'] is None:
+            raise ValueError("The 'device' argument is required for ImagingPlane.")
+        if args_to_set['excitation_lambda'] is None:
+            raise ValueError("The 'excitation_lambda' argument is required for ImagingPlane.")
+        if args_to_set['indicator'] is None:
+            raise ValueError("The 'indicator' argument is required for ImagingPlane.")
+        if args_to_set['location'] is None:
+            raise ValueError("The 'location' argument is required for ImagingPlane.")
+
         if args_to_set['manifold'] is not None:
             error_msg = "The 'manifold' argument is deprecated in favor of 'origin_coords' and 'grid_spacing'."
             self._error_on_new_pass_on_construct(error_msg=error_msg)

tests/unit/test_ophys.py

@@ -118,6 +118,7 @@ def test_init(self):
             grid_spacing_unit='gs_unit'
         )
         self.assertEqual(ip.optical_channel[0], oc)
+        self.assertEqual(ip.description, 'description')
         self.assertEqual(ip.device, device)
         self.assertEqual(ip.excitation_lambda, 600.)
         self.assertEqual(ip.imaging_rate, 300.)
@@ -196,6 +197,67 @@ def test_unit_deprecated(self):
         obj = ImagingPlane.__new__(ImagingPlane, in_construct_mode=True)
         obj.__init__(**kwargs)
 
+    def test_init_description_optional(self):
+        """Check creation of ImagingPlane with only required dependencies.
+
+        This is to check how creation of an ImagingPlane changes when the "description" argument moves from a
+        required arg in the middle of the required args section to an optional arg, in alignment with the schema.
+        """
+        oc, device = self.set_up_dependencies()
+
+        # description is now optional
+        ip = ImagingPlane(
+                name='test_imaging_plane',
+                optical_channel=oc,
+                device=device,
+                excitation_lambda=600.,
+                indicator='indicator',
+                location='location',
+            )
+        self.assertIsNone(ip.description)
+
+        # description is still required to be provided when using positional arguments
+        with warnings.catch_warnings(record=True):  # catch positional argument deprecation warning
+            with self.assertRaises(TypeError):
+                ImagingPlane(
+                    'test_imaging_plane',
+                    oc,
+                    device,
+                    600.,
+                    '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()
+
+        with self.assertRaisesWith(ValueError, "The 'device' argument is required for ImagingPlane."):
+            ImagingPlane(
+                name='test_imaging_plane',
+                optical_channel=oc,
+            )
+        with self.assertRaisesWith(ValueError, "The 'excitation_lambda' argument is required for ImagingPlane."):
+            ImagingPlane(
+                name='test_imaging_plane',
+                optical_channel=oc,
+                device=device,
+            )
+        with self.assertRaisesWith(ValueError, "The 'indicator' argument is required for ImagingPlane."):
+            ImagingPlane(
+                name='test_imaging_plane',
+                optical_channel=oc,
+                device=device,
+                excitation_lambda=600.,
+            )
+        with self.assertRaisesWith(ValueError, "The 'location' argument is required for ImagingPlane."):
+            ImagingPlane(
+                name='test_imaging_plane',
+                optical_channel=oc,
+                device=device,
+                excitation_lambda=600.,
+                indicator='indicator',
+            )
 
 class OnePhotonSeriesConstructor(TestCase):