From 2b8ba9bedbb552118b4977fc2508924f39e386d8 Mon Sep 17 00:00:00 2001 From: bendichter Date: Wed, 2 Feb 2022 15:38:16 -0400 Subject: [PATCH 1/9] update to use the latest dandi tools * rmv s3 utils * rmv corresponding tests * update tutorial to use latest dandi tools instead --- src/pynwb/base.py | 11 ++++++++++- tests/unit/test_base.py | 19 +++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/src/pynwb/base.py b/src/pynwb/base.py index 9924bc7bf..957b41203 100644 --- a/src/pynwb/base.py +++ b/src/pynwb/base.py @@ -1,8 +1,9 @@ from warnings import warn from collections.abc import Iterable -import numpy as np from typing import NamedTuple +import numpy as np + from hdmf.utils import docval, getargs, popargs, call_docval_func, get_docval from hdmf.common import DynamicTable, VectorData @@ -146,6 +147,14 @@ def __init__(self, **kwargs): "control", "control_description", "continuity") + + if ( + isinstance(kwargs["data"], (np.ndarray, list)) + and isinstance(kwargs["timestamps"], (np.ndarray, list)) + and not len(kwargs["data"]) == len(kwargs["timestamps"]) + ): + warn("Length of data does not match length of timestamps. Your data may be transposed. Time should be on " + "the 0th dimension") for key in keys: val = kwargs.get(key) if val is not None: diff --git a/tests/unit/test_base.py b/tests/unit/test_base.py index dc0869f0d..900b63e63 100644 --- a/tests/unit/test_base.py +++ b/tests/unit/test_base.py @@ -1,3 +1,5 @@ +import warnings + import numpy as np from pynwb.base import ProcessingModule, TimeSeries, Images, Image, TimeSeriesReferenceVectorData, TimeSeriesReference @@ -203,6 +205,23 @@ def test_conflicting_time_args(self): TimeSeries('test_ts2', [10, 11, 12, 13, 14, 15], 'grams', starting_time=30., timestamps=[.3, .4, .5, .6, .7, .8]) + def test_dimension_warning(self): + with warnings.catch_warnings(record=True) as w: + # Cause all warnings to always be triggered. + warnings.simplefilter("always") + TimeSeries( + name='test_ts2', + data=[10, 11, 12], + unit='grams', + timestamps=[.3, .4, .5, .6, .7, .8], + ) + assert len(w) == 1 + assert ( + "Length of data does not match length of timestamps. Your data may be " + "transposed. Time should be on the 0th dimension" + ) in str(w[-1].message) + + class TestImage(TestCase): From d2e971a9f3d7bcad934b9355b5ecf11ca34c0714 Mon Sep 17 00:00:00 2001 From: bendichter Date: Wed, 2 Feb 2022 15:40:21 -0400 Subject: [PATCH 2/9] flake8 --- tests/unit/test_base.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/unit/test_base.py b/tests/unit/test_base.py index 900b63e63..3f14585f3 100644 --- a/tests/unit/test_base.py +++ b/tests/unit/test_base.py @@ -222,7 +222,6 @@ def test_dimension_warning(self): ) in str(w[-1].message) - class TestImage(TestCase): def test_image(self): From a7122c4468af9d0e3732298466afa4a6f5aa5a0e Mon Sep 17 00:00:00 2001 From: bendichter Date: Wed, 2 Feb 2022 15:59:16 -0400 Subject: [PATCH 3/9] add ecephys checks --- src/pynwb/ecephys.py | 13 +++++++++++++ tests/unit/test_ecephys.py | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/src/pynwb/ecephys.py b/src/pynwb/ecephys.py index 91160753d..1dfab60f6 100644 --- a/src/pynwb/ecephys.py +++ b/src/pynwb/ecephys.py @@ -1,7 +1,9 @@ from collections.abc import Iterable +import warnings from hdmf.utils import docval, getargs, popargs, call_docval_func, get_docval from hdmf.data_utils import DataChunkIterator, assertEqualShape +import numpy as np from . import register_class, CORE_NAMESPACE from .base import TimeSeries @@ -78,6 +80,17 @@ class ElectricalSeries(TimeSeries): def __init__(self, **kwargs): name, electrodes, data, channel_conversion, filtering = popargs('name', 'electrodes', 'data', 'channel_conversion', 'filtering', kwargs) + if ( + isinstance(data, np.ndarray) + and not data.shape[1] == len(electrodes.data) + ): + if len(data) == len(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.") + else: + warnings.warn("The second dimension of data does not match the length of electrodes. Your data may be " + "transposed.") + super(ElectricalSeries, self).__init__(name, data, 'volts', **kwargs) self.electrodes = electrodes self.channel_conversion = channel_conversion diff --git a/tests/unit/test_ecephys.py b/tests/unit/test_ecephys.py index e9048da6c..1b934d9be 100644 --- a/tests/unit/test_ecephys.py +++ b/tests/unit/test_ecephys.py @@ -1,3 +1,5 @@ +import warnings + import numpy as np from pynwb.ecephys import ElectricalSeries, SpikeEventSeries, EventDetection, Clustering, EventWaveform,\ @@ -66,6 +68,38 @@ def test_invalid_data_shape(self): "2)', expected '((None,), (None, None), (None, None, None))')")): ElectricalSeries('test_ts1', np.ones((2, 2, 2, 2)), region, timestamps=[0.0, 0.1, 0.2, 0.3, 0.4, 0.5]) + def test_dimensions_warning(self): + table = make_electrode_table() + region = DynamicTableRegion('electrodes', [0, 2], 'the first and third electrodes', table) + with warnings.catch_warnings(record=True) as w: + # Cause all warnings to always be triggered. + warnings.simplefilter("always") + ElectricalSeries( + name="test_ts1", + data=np.ones((6, 3)), + electrodes=region, + timestamps=[0.0, 0.1, 0.2, 0.3, 0.4, 0.5] + ) + self.assertEqual(len(w), 1) + assert ( + "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: + # Cause all warnings to always be triggered. + warnings.simplefilter("always") + ElectricalSeries( + name="test_ts1", + data=np.ones((2, 6)), + electrodes=region, + rate=30000., + ) + 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." + ) in str(w[-1].message) + class SpikeEventSeriesConstructor(TestCase): From dae3c1c25e178f97b07d826b3d536463608bb542 Mon Sep 17 00:00:00 2001 From: bendichter Date: Wed, 2 Feb 2022 16:10:48 -0400 Subject: [PATCH 4/9] add ophys checks --- src/pynwb/ophys.py | 10 ++++++++++ tests/unit/test_ophys.py | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/src/pynwb/ophys.py b/src/pynwb/ophys.py index 97e3b2468..4fe0cff57 100644 --- a/src/pynwb/ophys.py +++ b/src/pynwb/ophys.py @@ -342,6 +342,16 @@ class RoiResponseSeries(TimeSeries): 'comments', 'description', 'control', 'control_description')) def __init__(self, **kwargs): rois = popargs('rois', kwargs) + if ( + isinstance(kwargs["data"], np.ndarray) + and not kwargs["data"].shape[1] == len(rois.data) + ): + if len(kwargs["data"]) == len(rois.data): + 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.") + else: + warnings.warn("The second dimension of data does not match the length of rois. Your data may be " + "transposed.") call_docval_func(super(RoiResponseSeries, self).__init__, kwargs) self.rois = rois diff --git a/tests/unit/test_ophys.py b/tests/unit/test_ophys.py index 87e40a421..09d235f78 100644 --- a/tests/unit/test_ophys.py +++ b/tests/unit/test_ophys.py @@ -1,3 +1,5 @@ +import warnings + import numpy as np from pynwb.base import TimeSeries @@ -289,6 +291,42 @@ def test_init(self): self.assertEqual(ts.unit, 'unit') self.assertEqual(ts.rois, rt_region) + def test_warnings(self): + ps = create_plane_segmentation() + rt_region = ps.create_roi_table_region(description='the second ROI', 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, 3)), + rois=rt_region, + unit="n.a.", + timestamps=[0.0, 0.1, 0.2, 0.3, 0.4, 0.5] + ) + self.assertEqual(len(w), 1) + assert ( + "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: + # Cause all warnings to always be triggered. + warnings.simplefilter("always") + RoiResponseSeries( + name="test_ts1", + data=np.ones((2, 6)), + rois=rt_region, + rate=30000., + unit="n.a.", + ) + 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." + ) in str(w[-1].message) + + class DfOverFConstructor(TestCase): def test_init(self): From 4e820322e0fe76e026b29ab57c31f1833725faac Mon Sep 17 00:00:00 2001 From: bendichter Date: Wed, 2 Feb 2022 16:12:55 -0400 Subject: [PATCH 5/9] flake8 --- tests/unit/test_ophys.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/unit/test_ophys.py b/tests/unit/test_ophys.py index 09d235f78..2cca3fab7 100644 --- a/tests/unit/test_ophys.py +++ b/tests/unit/test_ophys.py @@ -293,7 +293,7 @@ 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 second ROI', region=[0, 1]) with warnings.catch_warnings(record=True) as w: # Cause all warnings to always be triggered. @@ -322,12 +322,11 @@ 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." + "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) - class DfOverFConstructor(TestCase): def test_init(self): ps = create_plane_segmentation() From 29aeb3dbe865b86bedb17ff33db0c712c0a572ea Mon Sep 17 00:00:00 2001 From: bendichter Date: Thu, 3 Feb 2022 10:50:22 -0400 Subject: [PATCH 6/9] use get_data_shape --- src/pynwb/base.py | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/pynwb/base.py b/src/pynwb/base.py index 957b41203..a43b22d54 100644 --- a/src/pynwb/base.py +++ b/src/pynwb/base.py @@ -6,6 +6,7 @@ from hdmf.utils import docval, getargs, popargs, call_docval_func, get_docval from hdmf.common import DynamicTable, VectorData +from hdmf.utils import get_data_shape from . import register_class, CORE_NAMESPACE from .core import NWBDataInterface, MultiContainerInterface, NWBData @@ -148,10 +149,20 @@ def __init__(self, **kwargs): "control_description", "continuity") + data_shape = get_data_shape(data=kwargs["data"], strict_no_data_load=True) + timestamps_shape = get_data_shape(data=kwargs["timestamps"], strict_no_data_load=True) if ( - isinstance(kwargs["data"], (np.ndarray, list)) - and isinstance(kwargs["timestamps"], (np.ndarray, list)) - and not len(kwargs["data"]) == len(kwargs["timestamps"]) + # check that the shape is known + data_shape is not None and timestamps_shape is not None + + # check for scalars. Should never happen + and (len(data_shape) > 0 and len(timestamps_shape) > 0) + + # check that the length of the first dimension is known + and (data_shape[0] is not None and timestamps_shape[0] is not None) + + # check that the data and timestamps match + and (data_shape[0] != timestamps_shape[0]) ): warn("Length of data does not match length of timestamps. Your data may be transposed. Time should be on " "the 0th dimension") From 8639da4d709ef23631e6e7cc88899946ba239495 Mon Sep 17 00:00:00 2001 From: bendichter Date: Thu, 3 Feb 2022 10:56:13 -0400 Subject: [PATCH 7/9] use get_data_shape for ecephys --- src/pynwb/ecephys.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/pynwb/ecephys.py b/src/pynwb/ecephys.py index 1dfab60f6..d94170528 100644 --- a/src/pynwb/ecephys.py +++ b/src/pynwb/ecephys.py @@ -3,6 +3,7 @@ from hdmf.utils import docval, getargs, popargs, call_docval_func, get_docval from hdmf.data_utils import DataChunkIterator, assertEqualShape +from hdmf.utils import get_data_shape import numpy as np from . import register_class, CORE_NAMESPACE @@ -80,11 +81,13 @@ class ElectricalSeries(TimeSeries): def __init__(self, **kwargs): name, electrodes, data, channel_conversion, filtering = popargs('name', 'electrodes', 'data', 'channel_conversion', 'filtering', kwargs) + data_shape = get_data_shape(data, strict_no_data_load=True) if ( - isinstance(data, np.ndarray) - and not data.shape[1] == len(electrodes.data) + data_shape is not None + and len(data_shape) == 2 + and data_shape[1] != len(electrodes.data) ): - if len(data) == len(electrodes.data): + if data_shape[0] == len(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.") else: From be70eaf3db36c9194921bc912c27bae3625e76fc Mon Sep 17 00:00:00 2001 From: bendichter Date: Thu, 3 Feb 2022 11:03:45 -0400 Subject: [PATCH 8/9] use get_data_shape for ophys flake8 --- src/pynwb/ecephys.py | 1 - src/pynwb/ophys.py | 18 ++++++++++++++---- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/pynwb/ecephys.py b/src/pynwb/ecephys.py index d94170528..33a0783a9 100644 --- a/src/pynwb/ecephys.py +++ b/src/pynwb/ecephys.py @@ -4,7 +4,6 @@ from hdmf.utils import docval, getargs, popargs, call_docval_func, get_docval from hdmf.data_utils import DataChunkIterator, assertEqualShape from hdmf.utils import get_data_shape -import numpy as np from . import register_class, CORE_NAMESPACE from .base import TimeSeries diff --git a/src/pynwb/ophys.py b/src/pynwb/ophys.py index 4fe0cff57..24a2a6581 100644 --- a/src/pynwb/ophys.py +++ b/src/pynwb/ophys.py @@ -2,7 +2,7 @@ import numpy as np import warnings -from hdmf.utils import docval, getargs, popargs, call_docval_func, get_docval +from hdmf.utils import docval, getargs, popargs, call_docval_func, get_docval, get_data_shape from . import register_class, CORE_NAMESPACE from .base import TimeSeries @@ -342,11 +342,21 @@ class RoiResponseSeries(TimeSeries): 'comments', 'description', 'control', 'control_description')) def __init__(self, **kwargs): rois = popargs('rois', kwargs) + + data_shape = get_data_shape(data=kwargs["data"], strict_no_data_load=True) + rois_shape = get_data_shape(data=rois.data, strict_no_data_load=True) if ( - isinstance(kwargs["data"], np.ndarray) - and not kwargs["data"].shape[1] == len(rois.data) + data_shape is not None and rois_shape is not None + + # check that data is 2d and rois is 1d + and len(data_shape) == 2 and len(rois_shape) == 1 + + # 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 ): - if len(kwargs["data"]) == len(rois.data): + 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.") else: From c81d238fa5e93d8b91ad9cbc1587b9c5feb4278b Mon Sep 17 00:00:00 2001 From: Ben Dichter Date: Mon, 7 Feb 2022 15:28:25 -0400 Subject: [PATCH 9/9] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9c2b13c50..87e0b0f6a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ ### Minor improvements: - Improve constructor docstrings for Image types. @weiglszonja (#1418) +- Add checks for data orientation in ``TimeSeries``, ``ElectricalSeries``, and ``RoiResponseSeries`` @bendichter (#1428) ## PyNWB 2.0.0 (August 13, 2021)