Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make ImagingPlane.description optional #2051

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from
Open

Make ImagingPlane.description optional #2051

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b8a12fe
Make ImagingPlane.description optional
rly Mar 4, 2025
414b51c
Update changelog
rly Mar 4, 2025
ed8965a
Update CHANGELOG.md
rly Mar 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# PyNWB Changelog

## [Upcoming]

### Bug fixes
- Made `ImagingPlane.description` optional to conform with the NWB Schema. Note that if you use positional arguments to create
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a breaking change since the order of positional arguments was changed, correct?

Would it be better to take the approach that was used in add_electrode to prevent breaking changes until positional arguments are deprecated in the next major release?

an `ImagingPlane`, the order of arguments has changed. @rly [#2051](https://github.com/NeurodataWithoutBorders/pynwb/pull/2051)

## PyNWB 3.0.0 (February 26, 2025)

### Breaking changes
Expand Down
2 changes: 1 addition & 1 deletion src/pynwb/ophys.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ 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': 'description', 'type': str, 'doc': 'Description of this ImagingPlane.', 'default': None},
{'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
Expand Down
20 changes: 20 additions & 0 deletions tests/unit/test_ophys.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.)
Expand Down Expand Up @@ -196,6 +197,25 @@ def test_unit_deprecated(self):
obj = ImagingPlane.__new__(ImagingPlane, in_construct_mode=True)
obj.__init__(**kwargs)

def test_init_pos_args(self):
"""Check creation of ImagingPlane with only required dependencies and positional kwargs.

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()

ip = ImagingPlane(
'test_imaging_plane',
oc,
device,
600.,
'indicator',
'location',
)
self.assertIsNone(ip.description)
self.assertIs(ip.device, device)


class OnePhotonSeriesConstructor(TestCase):

Expand Down
Loading