From 68a1dc126875d3a99d4d700d6c3b1697e42e2163 Mon Sep 17 00:00:00 2001 From: Heberto Mayorquin Date: Wed, 20 Mar 2024 19:09:03 -0600 Subject: [PATCH 1/3] add validation --- src/pynwb/ndx_binned_spikes/__init__.py | 88 ++++++++++++++++++- src/pynwb/tests/test_binned_aligned_spikes.py | 23 ++++- 2 files changed, 108 insertions(+), 3 deletions(-) diff --git a/src/pynwb/ndx_binned_spikes/__init__.py b/src/pynwb/ndx_binned_spikes/__init__.py index c232e06..3afa00d 100644 --- a/src/pynwb/ndx_binned_spikes/__init__.py +++ b/src/pynwb/ndx_binned_spikes/__init__.py @@ -1,5 +1,9 @@ import os +import numpy as np + from pynwb import load_namespaces, get_class +from pynwb.core import NWBDataInterface +from hdmf.utils import docval, popargs_to_dict, get_docval, popargs try: from importlib.resources import files @@ -18,7 +22,89 @@ # Load the namespace load_namespaces(str(__spec_path)) -BinnedAlignedSpikes = get_class("BinnedAlignedSpikes", "ndx-binned-spikes") +# BinnedAlignedSpikes = get_class("BinnedAlignedSpikes", "ndx-binned-spikes") + +from pynwb import register_class, docval + + +@register_class(neurodata_type="BinnedAlignedSpikes", namespace="ndx-binned-spikes") +class BinnedAlignedSpikes(NWBDataInterface): + __nwbfields__ = ( + "name", + "bin_width_in_milliseconds", + "milliseconds_from_event_to_first_bin", + "data", + "event_timestamps", + "units", + ) + + DEFAULT_NAME = "BinnedAlignedSpikes" + + @docval( + { + "name": "name", + "type": str, + "doc": "The name of this container", + "default": DEFAULT_NAME, + }, + { + "name": "bin_width_in_milliseconds", + "type": float, + "doc": "The length in milliseconds of the bins", + }, + { + "name": "milliseconds_from_event_to_first_bin", + "type": float, + "doc": ( + "The time in milliseconds from the event (e.g. a stimuli or the beginning of a trial)," + "to the first bin. Note that this is a negative number if the first bin is before the event." + ), + "default": 0.0, + }, + { + "name": "data", + "type": "array_data", + "shape": [(None, None, None), (None, None)], + "doc": "The source of the data", + }, + { + "name": "event_timestamps", + "type": "array_data", + "doc": "The timestamps at which the event occurred.", + "shape": (None,), + }, + { + "name": "units", + "type": ("DynamicTableRegion"), + "doc": "A reference to the Units table region that contains the units of the data.", + "default": None, + }, + ) + def __init__(self, **kwargs): + + keys_to_set = ("bin_width_in_milliseconds", "milliseconds_from_event_to_first_bin", "units") + args_to_set = popargs_to_dict(keys_to_set, kwargs) + + keys_to_process = ("data", "event_timestamps") # these are properties and cannot be set with setattr + args_to_process = popargs_to_dict(keys_to_process, kwargs) + super().__init__(**kwargs) + + # Set the values + for key, val in args_to_set.items(): + setattr(self, key, val) + + # Post-process / post_init + data = args_to_process["data"] + + data = data if data.ndim == 3 else data[np.newaxis, ...] + + event_timestamps = args_to_process["event_timestamps"] + + if data.shape[1] != event_timestamps.shape[0]: + raise ValueError("The number of event timestamps must match the number of event repetitions in the data.") + + self.fields["data"] = data + self.fields["event_timestamps"] = event_timestamps # Remove these functions from the package diff --git a/src/pynwb/tests/test_binned_aligned_spikes.py b/src/pynwb/tests/test_binned_aligned_spikes.py index 62b0869..3af8e25 100644 --- a/src/pynwb/tests/test_binned_aligned_spikes.py +++ b/src/pynwb/tests/test_binned_aligned_spikes.py @@ -24,9 +24,9 @@ def setUp(self): self.number_of_event_repetitions = 4 self.bin_width_in_milliseconds = 20.0 self.milliseconds_from_event_to_first_bin = -100.0 - rng = np.random.default_rng(seed=0) + self.rng = np.random.default_rng(seed=0) - self.data = rng.integers( + self.data = self.rng.integers( low=0, high=100, size=( @@ -99,6 +99,25 @@ def test_constructor_units_region(self): expected_names = [unit_name_a, unit_name_c] self.assertListEqual(unit_table_names, expected_names) + def test_accepting_input_with_no_number_of_units_dimension(self): + + data = self.rng.integers( + low=0, + high=100, + size=( + self.number_of_event_repetitions, + self.number_of_bins, + ), + ) + binned_aligned_spikes = BinnedAlignedSpikes( + bin_width_in_milliseconds=self.bin_width_in_milliseconds, + milliseconds_from_event_to_first_bin=self.milliseconds_from_event_to_first_bin, + data=data, + event_timestamps=self.event_timestamps, + ) + + self.assertEqual(binned_aligned_spikes.data.shape, (1, self.number_of_event_repetitions, self.number_of_bins)) + class TestBinnedAlignedSpikesSimpleRoundtrip(TestCase): """Simple roundtrip test for BinnedAlignedSpikes.""" From 7026f5284185f91e7f56b05d634dc19f606bee6d Mon Sep 17 00:00:00 2001 From: Heberto Mayorquin Date: Wed, 20 Mar 2024 19:26:03 -0600 Subject: [PATCH 2/3] ruff --- src/pynwb/ndx_binned_spikes/__init__.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/pynwb/ndx_binned_spikes/__init__.py b/src/pynwb/ndx_binned_spikes/__init__.py index 3afa00d..41f2583 100644 --- a/src/pynwb/ndx_binned_spikes/__init__.py +++ b/src/pynwb/ndx_binned_spikes/__init__.py @@ -2,8 +2,9 @@ import numpy as np from pynwb import load_namespaces, get_class +from pynwb import register_class from pynwb.core import NWBDataInterface -from hdmf.utils import docval, popargs_to_dict, get_docval, popargs +from hdmf.utils import docval, popargs_to_dict try: from importlib.resources import files @@ -24,10 +25,8 @@ # BinnedAlignedSpikes = get_class("BinnedAlignedSpikes", "ndx-binned-spikes") -from pynwb import register_class, docval - -@register_class(neurodata_type="BinnedAlignedSpikes", namespace="ndx-binned-spikes") +@register_class(neurodata_type="BinnedAlignedSpikes", namespace="ndx-binned-spikes") #noqa class BinnedAlignedSpikes(NWBDataInterface): __nwbfields__ = ( "name", From 583edd87bd9822946e197858dd856d9b4f4c6674 Mon Sep 17 00:00:00 2001 From: Heberto Mayorquin Date: Thu, 21 Mar 2024 15:07:07 -0600 Subject: [PATCH 3/3] revert acceptance of data with no units and add test for timestamps assertion --- src/pynwb/ndx_binned_spikes/__init__.py | 7 ++--- src/pynwb/tests/test_binned_aligned_spikes.py | 29 +++++++------------ 2 files changed, 12 insertions(+), 24 deletions(-) diff --git a/src/pynwb/ndx_binned_spikes/__init__.py b/src/pynwb/ndx_binned_spikes/__init__.py index 41f2583..0f43b6e 100644 --- a/src/pynwb/ndx_binned_spikes/__init__.py +++ b/src/pynwb/ndx_binned_spikes/__init__.py @@ -63,7 +63,7 @@ class BinnedAlignedSpikes(NWBDataInterface): { "name": "data", "type": "array_data", - "shape": [(None, None, None), (None, None)], + "shape": [(None, None, None)], "doc": "The source of the data", }, { @@ -84,7 +84,7 @@ def __init__(self, **kwargs): keys_to_set = ("bin_width_in_milliseconds", "milliseconds_from_event_to_first_bin", "units") args_to_set = popargs_to_dict(keys_to_set, kwargs) - keys_to_process = ("data", "event_timestamps") # these are properties and cannot be set with setattr + keys_to_process = ("data", "event_timestamps") args_to_process = popargs_to_dict(keys_to_process, kwargs) super().__init__(**kwargs) @@ -94,9 +94,6 @@ def __init__(self, **kwargs): # Post-process / post_init data = args_to_process["data"] - - data = data if data.ndim == 3 else data[np.newaxis, ...] - event_timestamps = args_to_process["event_timestamps"] if data.shape[1] != event_timestamps.shape[0]: diff --git a/src/pynwb/tests/test_binned_aligned_spikes.py b/src/pynwb/tests/test_binned_aligned_spikes.py index 3af8e25..e1caf56 100644 --- a/src/pynwb/tests/test_binned_aligned_spikes.py +++ b/src/pynwb/tests/test_binned_aligned_spikes.py @@ -99,25 +99,16 @@ def test_constructor_units_region(self): expected_names = [unit_name_a, unit_name_c] self.assertListEqual(unit_table_names, expected_names) - def test_accepting_input_with_no_number_of_units_dimension(self): - - data = self.rng.integers( - low=0, - high=100, - size=( - self.number_of_event_repetitions, - self.number_of_bins, - ), - ) - binned_aligned_spikes = BinnedAlignedSpikes( - bin_width_in_milliseconds=self.bin_width_in_milliseconds, - milliseconds_from_event_to_first_bin=self.milliseconds_from_event_to_first_bin, - data=data, - event_timestamps=self.event_timestamps, - ) - - self.assertEqual(binned_aligned_spikes.data.shape, (1, self.number_of_event_repetitions, self.number_of_bins)) - + def test_constructor_inconsistent_timestamps_and_data_error(self): + shorter_timestamps = self.event_timestamps[:-1] + + with self.assertRaises(ValueError): + BinnedAlignedSpikes( + bin_width_in_milliseconds=self.bin_width_in_milliseconds, + milliseconds_from_event_to_first_bin=self.milliseconds_from_event_to_first_bin, + data=self.data, + event_timestamps=shorter_timestamps, + ) class TestBinnedAlignedSpikesSimpleRoundtrip(TestCase): """Simple roundtrip test for BinnedAlignedSpikes."""