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.

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

Allow Images in stimulus templates #1638

Merged
merged 4 commits into from
Jan 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
- Updated the [images tutorial](https://pynwb.readthedocs.io/en/stable/tutorials/domain/images.html) to provide example usage of an ``IndexSeries``
with a reference to ``Images``. @bendichter [#1602](https://github.com/NeurodataWithoutBorders/pynwb/pull/1602)
- Fixed an issue with the `tox` tool when upgrading to tox 4. @rly [#1608](https://github.com/NeurodataWithoutBorders/pynwb/pull/1608)
- Fixed an issue where `Images` were not allowed as stimulus templates. @rly [#1638](https://github.com/NeurodataWithoutBorders/pynwb/pull/1638)

## PyNWB 2.2.0 (October 19, 2022)

Expand Down
5 changes: 3 additions & 2 deletions src/pynwb/file.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
from .icephys import (IntracellularElectrode, SweepTable, PatchClampSeries, IntracellularRecordingsTable,
SimultaneousRecordingsTable, SequentialRecordingsTable, RepetitionsTable,
ExperimentalConditionsTable)
from .image import Images
from .ophys import ImagingPlane
from .ogen import OptogeneticStimulusSite
from .misc import Units
Expand Down Expand Up @@ -156,7 +157,7 @@ class NWBFile(MultiContainerInterface):
{
'attr': 'stimulus_template',
'add': '_add_stimulus_template_internal',
'type': TimeSeries,
'type': (TimeSeries, Images),
Copy link
Contributor

Choose a reason for hiding this comment

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

So you can add Images but not Image directly? I'm OK with that I just wanted to confirm that this is what we want.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct

'get': 'get_stimulus_template'
},
{
Expand Down Expand Up @@ -838,7 +839,7 @@ def add_stimulus(self, **kwargs):
if use_sweep_table:
self._update_sweep_table(timeseries)

@docval({'name': 'timeseries', 'type': TimeSeries},
@docval({'name': 'timeseries', 'type': (TimeSeries, Images)},
{'name': 'use_sweep_table', 'type': bool, 'default': False, 'doc': 'Use the deprecated SweepTable'})
def add_stimulus_template(self, **kwargs):
timeseries = popargs('timeseries', kwargs)
Expand Down
2 changes: 1 addition & 1 deletion src/pynwb/testing/__init__.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
from hdmf.testing import TestCase, H5RoundTripMixin
from .testh5io import NWBH5IOMixin, AcquisitionH5IOMixin
from .testh5io import NWBH5IOMixin, AcquisitionH5IOMixin, NWBH5IOFlexMixin
from .utils import remove_test_file
from .icephys_testutils import create_icephys_stimulus_and_response, create_icephys_testfile

Expand Down
187 changes: 182 additions & 5 deletions src/pynwb/testing/testh5io.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,10 @@ def setUp(self):
self.container_type = self.container.__class__.__name__
self.filename = 'test_%s.nwb' % self.container_type
self.export_filename = 'test_export_%s.nwb' % self.container_type
self.writer = None
self.reader = None
self.export_reader = None

def tearDown(self):
if self.writer is not None:
self.writer.close()
if self.reader is not None:
self.reader.close()
if self.export_reader is not None:
Expand Down Expand Up @@ -85,9 +82,14 @@ def test_roundtrip_export(self):
def roundtripContainer(self, cache_spec=False):
"""Add the Container to an NWBFile, write it to file, read the file, and return the Container from the file.
"""
description = 'a file to test writing and reading a %s' % self.container_type
session_description = 'a file to test writing and reading a %s' % self.container_type
identifier = 'TEST_%s' % self.container_type
nwbfile = NWBFile(description, identifier, self.start_time, file_create_date=self.create_date)
nwbfile = NWBFile(
session_description=session_description,
identifier=identifier,
session_start_time=self.start_time,
file_create_date=self.create_date
)
self.addContainer(nwbfile)

with warnings.catch_warnings(record=True) as ws:
Expand Down Expand Up @@ -199,3 +201,178 @@ def addContainer(self, nwbfile):
def getContainer(self, nwbfile):
''' Get the NWBDataInterface object from the file '''
return nwbfile.get_acquisition(self.container.name)


class NWBH5IOFlexMixin(metaclass=ABCMeta):
"""
Mixin class that includes methods to run a flexible roundtrip test.
The setUp, test_roundtrip, and tearDown methods will be run by unittest.

The abstract methods getContainerType, addContainer, and getContainer must be
implemented by classes that includethis mixin.

Example::

class TestMyContainerIO(NWBH5IOFlexMixin, TestCase):
def getContainerType(self):
# return the name of the type of Container being tested, for test ID purposes
def addContainer(self):
# add the test Container to the test NWB file
def getContainer(self, nwbfile):
# return the test Container from an NWB file

This class is more flexible than NWBH5IOMixin and should be used for new roundtrip tests.

This code is adapted from hdmf.testing.H5RoundTripMixin.
"""

def setUp(self):
container_type = self.getContainerType().replace(" ", "_")
session_description = 'A file to test writing and reading a %s' % container_type
identifier = 'TEST_%s' % container_type
session_start_time = datetime(1971, 1, 1, 12, tzinfo=tzutc())
self.nwbfile = NWBFile(
session_description=session_description,
identifier=identifier,
session_start_time=session_start_time,
)
self.addContainer()
self.container = self.getContainer(self.nwbfile)
self.filename = 'test_%s.nwb' % container_type
self.export_filename = 'test_export_%s.nwb' % container_type
self.reader = None
self.export_reader = None

def tearDown(self):
if self.reader is not None:
self.reader.close()
if self.export_reader is not None:
self.export_reader.close()
remove_test_file(self.filename)
remove_test_file(self.export_filename)

def getContainerType() -> str:
"""Return the name of the type of Container being tested, for test ID purposes."""
raise NotImplementedError('Cannot run test unless getContainerType is implemented.')

@abstractmethod
def addContainer(self):
"""Add the test Container to the NWBFile.

The NWBFile is accessible from self.nwbfile.
The Container should be accessible from getContainer(self.nwbfile).
"""
raise NotImplementedError('Cannot run test unless addContainer is implemented.')

@abstractmethod
def getContainer(self, nwbfile: NWBFile):
"""Return the test Container from the given NWBFile."""
raise NotImplementedError('Cannot run test unless getContainer is implemented.')

def test_roundtrip(self):
"""Test whether the read Container has the same contents as the original Container and validate the file.
"""
self.read_container = self.roundtripContainer()
self.assertIsNotNone(str(self.container)) # added as a test to make sure printing works
self.assertIsNotNone(str(self.read_container))
# make sure we get a completely new object
self.assertNotEqual(id(self.container), id(self.read_container))
# make sure the object ID is preserved
self.assertIs(self.read_nwbfile.objects[self.container.object_id], self.read_container)
self.assertContainerEqual(self.read_container, self.container)

def test_roundtrip_export(self):
"""
Test whether the test Container read from an exported file has the same contents as the original test Container
and validate the file.
"""
self.read_container = self.roundtripExportContainer() # this container is read from the exported file
self.assertIsNotNone(str(self.read_container)) # added as a test to make sure printing works
# make sure we get a completely new object
self.assertNotEqual(id(self.container), id(self.read_container))
# make sure the object ID is preserved
self.assertIs(self.read_exported_nwbfile.objects[self.container.object_id], self.read_container)
self.assertContainerEqual(self.read_container, self.container, ignore_hdmf_attrs=True)

def roundtripContainer(self, cache_spec=False):
"""Write the file, validate the file, read the file, and return the Container from the file.
"""

# catch all warnings
with warnings.catch_warnings(record=True) as ws:
with NWBHDF5IO(self.filename, mode='w') as write_io:
write_io.write(self.nwbfile, cache_spec=cache_spec)

self.validate()

# this is not closed until tearDown() or an exception from self.getContainer below
self.reader = NWBHDF5IO(self.filename, mode='r')
self.read_nwbfile = self.reader.read()

# parse warnings and raise exceptions for certain types of warnings
if ws:
for w in ws:
if issubclass(w.category, (MissingRequiredBuildWarning,
BrokenLinkWarning)):
raise Exception('%s: %s' % (w.category.__name__, w.message))
else:
warnings.warn(w.message, w.category)

try:
return self.getContainer(self.read_nwbfile)
except Exception as e:
self.reader.close()
self.reader = None
raise e

def roundtripExportContainer(self, cache_spec=False):
"""
Roundtrip the container, then export the read NWBFile to a new file, validate the files, and return the test
Container from the file.
"""
# write and read the file. self.reader will be set
self.roundtripContainer(cache_spec=cache_spec)

# catch all warnings
with warnings.catch_warnings(record=True) as ws:
NWBHDF5IO.export_io(
src_io=self.reader,
path=self.export_filename,
cache_spec=cache_spec,
)

self.validate()

# this is not closed until tearDown() or an exception from self.getContainer below
self.export_reader = NWBHDF5IO(self.export_filename, mode='r')
self.read_exported_nwbfile = self.export_reader.read()

# parse warnings and raise exceptions for certain types of warnings
if ws:
for w in ws:
if issubclass(w.category, (MissingRequiredBuildWarning,
BrokenLinkWarning)):
raise Exception('%s: %s' % (w.category.__name__, w.message))
else:
warnings.warn(w.message, w.category)

try:
return self.getContainer(self.read_exported_nwbfile)
except Exception as e:
self.export_reader.close()
self.export_reader = None
raise e

def validate(self):
"""Validate the created files."""
if os.path.exists(self.filename):
with NWBHDF5IO(self.filename, mode='r') as io:
errors = pynwb_validate(io)
if errors:
raise Exception("\n".join(errors))

if os.path.exists(self.export_filename):
with NWBHDF5IO(self.filename, mode='r') as io:
errors = pynwb_validate(io)
if errors:
raise Exception("\n".join(errors))
35 changes: 34 additions & 1 deletion tests/integration/hdf5/test_nwbfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,11 @@
from hdmf.common import DynamicTable

from pynwb import NWBFile, TimeSeries, NWBHDF5IO, get_manager
from pynwb.base import Image, Images
from pynwb.file import Subject
from pynwb.epoch import TimeIntervals
from pynwb.ecephys import ElectricalSeries
from pynwb.testing import NWBH5IOMixin, TestCase, remove_test_file
from pynwb.testing import NWBH5IOMixin, NWBH5IOFlexMixin, TestCase, remove_test_file


class TestNWBFileHDF5IO(TestCase):
Expand Down Expand Up @@ -586,3 +587,35 @@ def test_roundtrip(self):
super().test_roundtrip()
for ii, item in enumerate(self.read_container):
pd.testing.assert_frame_equal(self.table[ii+1], item)


class TestAddStimulusTemplateTimeSeries(NWBH5IOFlexMixin, TestCase):

def getContainerType(self):
return "time series stored as a stimulus template"

def addContainer(self):
ts = TimeSeries(
name="test_ts",
data=[0, 1, 2, 3, 4, 5],
unit="grams",
timestamps=[0.0, 0.1, 0.2, 0.3, 0.4, 0.5]
)
self.nwbfile.add_stimulus_template(ts)

def getContainer(self, nwbfile):
return nwbfile.get_stimulus_template("test_ts")


class TestAddStimulusTemplateImages(NWBH5IOFlexMixin, TestCase):

def getContainerType(self):
return "images stored as a stimulus template"

def addContainer(self):
image1 = Image(name="test_image1", data=np.ones((10, 10)))
images = Images(name="images_name", images=[image1])
self.nwbfile.add_stimulus_template(images)

def getContainer(self, nwbfile):
return nwbfile.get_stimulus_template("images_name")
7 changes: 7 additions & 0 deletions tests/unit/test_file.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from dateutil.tz import tzlocal, tzutc

from pynwb import NWBFile, TimeSeries, NWBHDF5IO
from pynwb.base import Image, Images
from pynwb.file import Subject, ElectrodeTable
from pynwb.epoch import TimeIntervals
from pynwb.ecephys import ElectricalSeries
Expand Down Expand Up @@ -153,6 +154,12 @@ def test_add_stimulus_template(self):
'grams', timestamps=[0.0, 0.1, 0.2, 0.3, 0.4, 0.5]))
self.assertEqual(len(self.nwbfile.stimulus_template), 1)

def test_add_stimulus_template_images(self):
image1 = Image(name='test_image1', data=np.ones((10, 10)))
images = Images(name='images_name', images=[image1])
self.nwbfile.add_stimulus_template(images)
self.assertEqual(len(self.nwbfile.stimulus_template), 1)

def test_add_analysis(self):
self.nwbfile.add_analysis(TimeSeries('test_ts', [0, 1, 2, 3, 4, 5],
'grams', timestamps=[0.0, 0.1, 0.2, 0.3, 0.4, 0.5]))
Expand Down