Fix RoiResponseSeries dim warning and clarify other dim warnings (#1491)

* Fix incorrect warning #1490

* Print type and name of object when warning about dims

Author: rly

CHANGELOG.md

@@ -3,7 +3,7 @@
 ## PyNWB 2.1.0 (Upcoming)
 
 ### Breaking changes:
-- Raise a warning if `SpatialSeries.data` has more than 3 columns (#1455, #1480)
+- A warning is now raised if `SpatialSeries.data` has more than 3 columns. @bendichter, @rly (#1455, #1480)
 - Updated ``TimeIntervals`` to use the new  ``TimeSeriesReferenceVectorData`` type. This does not alter the overall
   structure of ``TimeIntervals`` in a major way aside from changing the value of the ``neurodata_type`` attribute of the
   ``TimeIntervals.timeseries`` column from ``VectorData`` to ``TimeSeriesReferenceVectorData``. This change facilitates
@@ -26,30 +26,36 @@
   warnings when the constructor of a class mapped to an HDMF-common data type or an autogenerated data type class
   is passed positional arguments instead of all keyword arguments. @rly (#1484)
 - Moved logic that checks the 0th dimension of TimeSeries data equals the length of timestamps to a private method in the
-  ``TimeSeries`` class. This is to avoid raising a warning when an ImageSeries is used with external file. @weiglszonja (#1486)
+  ``TimeSeries`` class. This is to avoid raising a warning when an ImageSeries is used with external file.
+  @weiglszonja (#1486)
+- Improved warning text when dimensions are not matched in `TimeSeries`, `ElectricalSeries`, and `RoiResponseSeries`.
+  @rly (#1491)
 
 ### Documentation and tutorial enhancements:
 - Added tutorial on annotating data via ``TimeIntervals``. @oruebel (#1390)
-- Add copy button to code blocks @weiglszonja (#1460)
-- Create behavioral tutorial @weiglszonja (#1464)
-- Enhance display of icephys pandas tutorial by using ``dataframe_image`` to render and display large tables
+- Added copy button to code blocks @weiglszonja (#1460)
+- Created behavioral tutorial @weiglszonja (#1464)
+- Enhanced display of icephys pandas tutorial by using ``dataframe_image`` to render and display large tables
   as images. @oruebel (#1469)
-- Create tutorial about reading and exploring an existing `NWBFile` @weiglszonja (#1453)
+- Created tutorial about reading and exploring an existing `NWBFile` @weiglszonja (#1453)
 - Added new logo for PyNWB. @oruebel (#1461)
 - Minor text fixes. @oruebel @bendichter (#1443, #1462, #1463, #1466, #1472, #1473)
 
 ### Bug fixes:
 - Fixed input data types to allow only `float` for fields `conversion` and `offset` in definition of
   ``TimeSeries``. @codycbakerphd (#1424)
+- Fixed incorrect warning in `RoiResponseSeries.__init__` about mismatch between the second dimension of data and
+  the length of rois. @rly (#1491)
 
 
 ## PyNWB 2.0.1 (March 16, 2022)
 
 ### Bug fixes:
-- Add `environment-ros3.yml` to `MANIFEST.in` for inclusion in source distributions. @rly (#1398)
-- Fix bad error check in ``IntracellularRecordingsTable.add_recording`` when adding ``IZeroClampSeries``. @oruebel (#1410)
-- Skip ros3 tests if internet access or the ros3 driver are not available. @oruebel (#1414)
-- Fix CI issues. @rly (#1427)
+- Added `environment-ros3.yml` to `MANIFEST.in` for inclusion in source distributions. @rly (#1398)
+- Fixed bad error check in ``IntracellularRecordingsTable.add_recording`` when adding ``IZeroClampSeries``.
+  @oruebel (#1410)
+- Skipped ros3 tests if internet access or the ros3 driver are not available. @oruebel (#1414)
+- Fixed CI issues. @rly (#1427)
 
 ### Documentation and tutorial enhancements:
 - Enhanced ordering of sphinx gallery tutorials to use alphabetic ordering based on tutorial headings. @oruebel (#1399)
@@ -66,8 +72,8 @@
 - Minor text fixes. @bendichter (#1437, #1400)
 
 ### Minor improvements:
-- Improve constructor docstrings for Image types. @weiglszonja (#1418)
-- Add checks for data orientation in ``TimeSeries``, ``ElectricalSeries``, and ``RoiResponseSeries`` @bendichter (#1428)
+- Improved constructor docstrings for Image types. @weiglszonja (#1418)
+- Added checks for data orientation in ``TimeSeries``, ``ElectricalSeries``, and ``RoiResponseSeries`` @bendichter (#1428)
 - Added checks for data orientation in ``TimeSeries``, ``ElectricalSeries``, and ``RoiResponseSeries``.
   @bendichter (#1426)
 - Enhanced issue template forms on GitHub. @CodyCBakerPHD (#1434)

src/pynwb/base.py

@@ -190,8 +190,8 @@ def __init__(self, **kwargs):
             raise TypeError("either 'timestamps' or 'rate' must be specified")
 
         if not self._check_time_series_dimension():
-            warn("Length of data does not match length of timestamps. Your data may be transposed. Time should be on "
-                 "the 0th dimension")
+            warn("%s '%s': Length of data does not match length of timestamps. Your data may be transposed. "
+                 "Time should be on the 0th dimension" % (self.__class__.__name__, self.name))
 
     def _check_time_series_dimension(self):
         """Check that the 0th dimension of data equals the length of timestamps, when applicable.

src/pynwb/ecephys.py

@@ -85,11 +85,12 @@ def __init__(self, **kwargs):
             and data_shape[1] != len(args_to_set['electrodes'].data)
         ):
             if data_shape[0] == len(args_to_set['electrodes'].data):
-                warnings.warn("The second dimension of data does not match the length of electrodes, but instead the "
-                              "first does. Data is oriented incorrectly and should be transposed.")
+                warnings.warn("%s '%s': The second dimension of data does not match the length of electrodes, "
+                              "but instead the first does. Data is oriented incorrectly and should be transposed."
+                              % (self.__class__.__name__, kwargs["name"]))
             else:
-                warnings.warn("The second dimension of data does not match the length of electrodes. Your data may be "
-                              "transposed.")
+                warnings.warn("%s '%s': The second dimension of data does not match the length of electrodes. "
+                              "Your data may be transposed." % (self.__class__.__name__, kwargs["name"]))
 
         kwargs['unit'] = 'volts'  # fixed value
         super().__init__(**kwargs)

src/pynwb/image.py

@@ -81,8 +81,8 @@ def __init__(self, **kwargs):
 
         if not self._check_image_series_dimension():
             warnings.warn(
-                "Length of data does not match length of timestamps. "
-                "Your data may be transposed. Time should be on the 0th dimension"
+                "%s '%s': Length of data does not match length of timestamps. Your data may be transposed. "
+                "Time should be on the 0th dimension" % (self.__class__.__name__, self.name)
             )
 
     def _check_time_series_dimension(self):

src/pynwb/ophys.py

@@ -354,14 +354,15 @@ def __init__(self, **kwargs):
             # check that key dimensions are known
             and data_shape[1] is not None and rois_shape[0] is not None
 
-            and data_shape[1] != rois_shape
+            and data_shape[1] != rois_shape[0]
         ):
             if data_shape[0] == rois_shape[0]:
-                warnings.warn("The second dimension of data does not match the length of rois, but instead the "
-                              "first does. Data is oriented incorrectly and should be transposed.")
+                warnings.warn("%s '%s': The second dimension of data does not match the length of rois, "
+                              "but instead the first does. Data is oriented incorrectly and should be transposed."
+                              % (self.__class__.__name__, kwargs["name"]))
             else:
-                warnings.warn("The second dimension of data does not match the length of rois. Your data may be "
-                              "transposed.")
+                warnings.warn("%s '%s': The second dimension of data does not match the length of rois. "
+                              "Your data may be transposed." % (self.__class__.__name__, kwargs["name"]))
         super().__init__(**kwargs)
         self.rois = rois
 

tests/unit/test_base.py

@@ -375,7 +375,7 @@ def test_conflicting_time_args(self):
 
     def test_dimension_warning(self):
         msg = (
-            "Length of data does not match length of timestamps. Your data may be "
+            "TimeSeries 'test_ts2': Length of data does not match length of timestamps. Your data may be "
             "transposed. Time should be on the 0th dimension"
         )
         with self.assertWarnsWith(UserWarning, msg):

tests/unit/test_ecephys.py

@@ -85,7 +85,8 @@ def test_dimensions_warning(self):
             )
             self.assertEqual(len(w), 1)
             assert (
-                "The second dimension of data does not match the length of electrodes. Your data may be transposed."
+                "ElectricalSeries 'test_ts1': The second dimension of data does not match the length of electrodes. "
+                "Your data may be transposed."
                 ) in str(w[-1].message)
 
         with warnings.catch_warnings(record=True) as w:
@@ -99,8 +100,8 @@ def test_dimensions_warning(self):
             )
             self.assertEqual(len(w), 1)
             assert (
-               "The second dimension of data does not match the length of electrodes, but instead the first does. Data "
-               "is oriented incorrectly and should be transposed."
+               "ElectricalSeries 'test_ts1': The second dimension of data does not match the length of electrodes, "
+               "but instead the first does. Data is oriented incorrectly and should be transposed."
                    ) in str(w[-1].message)
 
 

tests/unit/test_image.py

@@ -79,7 +79,7 @@ def test_dimension_warning(self):
         """Test that a warning is raised when the dimensions of the data are not the
         same as the dimensions of the timestamps."""
         msg = (
-            "Length of data does not match length of timestamps. Your data may be "
+            "ImageSeries 'test_iS': Length of data does not match length of timestamps. Your data may be "
             "transposed. Time should be on the 0th dimension"
         )
         with self.assertWarnsWith(UserWarning, msg):

tests/unit/test_ophys.py

@@ -294,7 +294,19 @@ def test_init(self):
 
     def test_warnings(self):
         ps = create_plane_segmentation()
-        rt_region = ps.create_roi_table_region(description='the second ROI', region=[0, 1])
+        rt_region = ps.create_roi_table_region(description='the first two ROIs', region=[0, 1])
+
+        with warnings.catch_warnings(record=True) as w:
+            # Cause all warnings to always be triggered.
+            warnings.simplefilter("always")
+            RoiResponseSeries(
+                name="test_ts1",
+                data=np.ones((6, 2)),
+                rois=rt_region,
+                rate=30000.,
+                unit="n.a.",
+            )
+            self.assertEqual(w, [])
 
         with warnings.catch_warnings(record=True) as w:
             # Cause all warnings to always be triggered.
@@ -308,7 +320,8 @@ def test_warnings(self):
             )
             self.assertEqual(len(w), 1)
             assert (
-                    "The second dimension of data does not match the length of rois. Your data may be transposed."
+                    "RoiResponseSeries 'test_ts1': The second dimension of data does not match the length of rois. "
+                    "Your data may be transposed."
                 ) in str(w[-1].message)
 
         with warnings.catch_warnings(record=True) as w:
@@ -323,8 +336,8 @@ def test_warnings(self):
             )
             self.assertEqual(len(w), 1)
             assert (
-               "The second dimension of data does not match the length of rois, but instead the first does. "
-               "Data is oriented incorrectly and should be transposed."
+               "RoiResponseSeries 'test_ts1': The second dimension of data does not match the length of rois, "
+               "but instead the first does. Data is oriented incorrectly and should be transposed."
                    ) in str(w[-1].message)