diff --git a/CHANGELOG.md b/CHANGELOG.md index 606d6abe2..fd3c29024 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/src/pynwb/ophys.py b/src/pynwb/ophys.py index a5121f678..b68baf1c6 100644 --- a/src/pynwb/ophys.py +++ b/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) diff --git a/tests/unit/test_ophys.py b/tests/unit/test_ophys.py index b196dbda5..c52fae9fa 100644 --- a/tests/unit/test_ophys.py +++ b/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):