Make Units.waveform_unit configurable

CHANGELOG.md

@@ -2,7 +2,12 @@
 
 ## PyNWB 4.1.1 (Unreleased)
 
+### Changed
+- Added support for NWB Schema 2.10.1
+  - The `unit` attribute of `Units.waveform_mean`, `Units.waveform_sd`, and `Units.waveforms` now has a default value of `"volts"` instead of a fixed value of `"volts"`.
+
 ### Fixed
+- Fixed `Units.waveform_unit` having no effect on the written file. The `waveform_unit` passed to `Units` is now written to the `waveform_mean`, `waveform_sd`, and `waveforms` columns, and `"volts"` remains the default. PyNWB now also warns when the waveform columns of a file being read carry different `unit` or `sampling_rate` attributes, since only one value per attribute is kept on the `Units` container. @rly [#2162](https://github.com/NeurodataWithoutBorders/pynwb/issues/2162)
 - Fixed reading a file whose dates carry a sub-minute UTC offset (e.g. `1900-10-01T00:00:00-05:50:36`). @h-mayorquin [#2230](https://github.com/NeurodataWithoutBorders/pynwb/pull/2230)
 - Fixed wide pandas DataFrames in the tutorials spilling out of the content column and into the right margin. @bendichter [#2236](https://github.com/NeurodataWithoutBorders/pynwb/pull/2236)
 

docs/gallery/domain/ecephys.py

@@ -358,6 +358,20 @@
 # The :py:class:`~pynwb.misc.Units` table can contain simply the spike times of sorted units, or you can also include
 # individual and mean waveform information in some of the optional, predefined :py:class:`~pynwb.misc.Units` table
 # columns: ``waveform_mean``, ``waveform_sd``, or ``waveforms``.
+#
+# The sampling rate and unit of measurement of those three columns are set with the ``waveform_rate`` and
+# ``waveform_unit`` arguments of :py:class:`~pynwb.misc.Units`, which default to ``None`` and ``"volts"``.
+# Because both are constructor arguments, set them when you build the :py:class:`~pynwb.misc.Units` table and
+# assign it to :py:attr:`.NWBFile.units`::
+#
+#     from pynwb.misc import Units
+#
+#     nwbfile.units = Units(
+#         name="units",
+#         description="units table",
+#         waveform_rate=30000.0,
+#         waveform_unit="microvolts",
+#     )
 
 nwbfile.units.to_dataframe()
 

src/pynwb/io/misc.py

@@ -1,3 +1,5 @@
+import warnings
+
 from hdmf.common.io.table import DynamicTableMap
 
 from .. import register_map
@@ -22,17 +24,27 @@ def waveform_unit_carg(self, builder, manager):
         return self._get_waveform_stat(builder, 'unit')
 
     def _get_waveform_stat(self, builder, attribute):
+        """Get the value of an attribute shared by the waveform columns of a Units table.
+
+        The `Units` container holds one `waveform_rate` and one `waveform_unit` for the whole table, while the
+        file stores a `sampling_rate` and `unit` attribute on each waveform column. When the columns disagree,
+        the value of the first populated column is used and a warning is raised.
+        """
         waveform_columns = ('waveform_mean', 'waveform_sd', 'waveforms')
-        stats = [builder[column].attributes.get(attribute) for column in waveform_columns if column in builder]
-        if not stats:
+        stats = {column: builder[column].attributes.get(attribute)
+                 for column in waveform_columns if column in builder}
+        populated_stats = {column: value for column, value in stats.items() if value is not None}
+        if not populated_stats:
             return None
-        populated_stats = [stat for stat in stats if stat is not None]
-        if len(set(populated_stats)) > 1:
-            # throw warning
-            pass
-        if populated_stats:
-            return populated_stats[0]
-        return None
+        first_column, first_value = next(iter(populated_stats.items()))
+        if len(set(populated_stats.values())) > 1:
+            warnings.warn(
+                f"The '{attribute}' attribute differs across the waveform columns of Units "
+                f"'{builder.name}': {populated_stats}. Using the value of '{first_column}'.",
+                UserWarning,
+                stacklevel=2
+            )
+        return first_value
 
     @DynamicTableMap.object_attr("electrodes")
     def electrodes_column(self, container, manager):

src/pynwb/misc.py

@@ -180,9 +180,11 @@ class Units(DynamicTable):
             {'name': 'electrode_table', 'type': DynamicTable,
              'doc': 'the table that the *electrodes* column indexes', 'default': None},
             {'name': 'waveform_rate', 'type': float,
-             'doc': 'Sampling rate of the waveform means', 'default': None},
+             'doc': 'Sampling rate of the data in the waveform_mean, waveform_sd, and waveforms columns',
+             'default': None},
             {'name': 'waveform_unit', 'type': str,
-             'doc': 'Unit of measurement of the waveform means', 'default': 'volts'},
+             'doc': 'Unit of measurement of the data in the waveform_mean, waveform_sd, and waveforms columns',
+             'default': 'volts'},
             {'name': 'resolution', 'type': float,
              'doc': 'The smallest possible difference between two spike times', 'default': None},
             allow_positional=AllowPositional.WARNING,

tests/integration/hdf5/test_misc.py

@@ -2,9 +2,10 @@
 import numpy as np
 
 from hdmf.common import VectorData, DynamicTableRegion
-from pynwb import TimeSeries
+from pynwb import NWBHDF5IO, TimeSeries
 from pynwb.misc import Units, DecompositionSeries, FrequencyBandsTable
-from pynwb.testing import NWBH5IOMixin, AcquisitionH5IOMixin, TestCase
+from pynwb.testing import NWBH5IOMixin, AcquisitionH5IOMixin, TestCase, remove_test_file
+from pynwb.testing.mock.file import mock_NWBFile
 from pynwb.ecephys import ElectrodeGroup, ElectrodesTable
 from pynwb.device import Device
 
@@ -123,6 +124,77 @@ def test_waveforms_attributes_written(self):
             self.assertEqual(unit, 'volts')
 
 
+class TestUnitsCustomWaveformUnitIO(AcquisitionH5IOMixin, TestCase):
+    """Test roundtripping a waveform unit other than the default 'volts'."""
+
+    def setUpContainer(self):
+        ut = Units(
+            name='UnitsCustomWaveformUnitTest',
+            description='a simple table for testing a custom Units waveform unit',
+            waveform_rate=40000.,
+            waveform_unit='microvolts',
+        )
+        ut.add_unit(
+            spike_times=[0., 1., 2.],
+            waveform_mean=[1., 2., 3.],
+            waveform_sd=[4., 5., 6.],
+            waveforms=[
+                [  # elec 1
+                    [1, 2, 3],
+                    [1, 2, 3]
+                ], [  # elec 2
+                    [1, 2, 3],
+                    [1, 2, 3]
+                ]
+            ],
+        )
+        return ut
+
+    def test_waveform_unit_roundtrip(self):
+        ut = self.roundtripContainer()
+        self.assertEqual(ut.waveform_unit, 'microvolts')
+
+    def test_waveform_unit_written(self):
+        self.roundtripContainer()
+        with h5py.File(self.filename, 'r') as infile:
+            units = infile['acquisition'][self.container.name]
+            for column in ('waveform_mean', 'waveform_sd', 'waveforms'):
+                unit = units[column].attrs['unit']
+                if isinstance(unit, bytes):
+                    unit = unit.decode('utf-8')
+                self.assertEqual(unit, 'microvolts')
+
+
+class TestUnitsMismatchedWaveformUnit(TestCase):
+    """Test reading a file whose waveform columns carry different unit attributes."""
+
+    def setUp(self):
+        self.filename = 'test_units_mismatched_waveform_unit.nwb'
+        nwbfile = mock_NWBFile()
+        ut = Units(name='units', description='a table for testing mismatched waveform units')
+        ut.add_unit(
+            spike_times=[0., 1., 2.],
+            waveform_mean=[1., 2., 3.],
+            waveform_sd=[4., 5., 6.],
+        )
+        nwbfile.units = ut
+        with NWBHDF5IO(self.filename, 'w') as io:
+            io.write(nwbfile)
+        with h5py.File(self.filename, 'r+') as infile:
+            infile['units']['waveform_sd'].attrs['unit'] = 'microvolts'
+
+    def tearDown(self):
+        remove_test_file(self.filename)
+
+    def test_warn_on_mismatched_unit(self):
+        msg = ("The 'unit' attribute differs across the waveform columns of Units 'units': "
+               "{'waveform_mean': 'volts', 'waveform_sd': 'microvolts'}. Using the value of 'waveform_mean'.")
+        with self.assertWarnsWith(UserWarning, msg):
+            with NWBHDF5IO(self.filename, 'r') as io:
+                nwbfile = io.read()
+                self.assertEqual(nwbfile.units.waveform_unit, 'volts')
+
+
 class TestUnitsFileIO(NWBH5IOMixin, TestCase):
 
     def setUpContainer(self):

tests/unit/test_misc.py

@@ -323,6 +323,10 @@ def test_waveform_attrs(self):
         self.assertEqual(ut.waveform_rate, 40000.)
         self.assertEqual(ut.waveform_unit, 'volts')
 
+    def test_custom_waveform_unit(self):
+        ut = Units(waveform_unit='microvolts')
+        self.assertEqual(ut.waveform_unit, 'microvolts')
+
     def test_get_starting_time(self):
         """Test get_starting_time returns the earliest spike time across units."""
         ut = Units()