improvements after Ben review (#11)

README.md

@@ -66,7 +66,7 @@ ecephys_processing_module.add(binned_aligned_spikes)
 ### Parameters and data structure
 The structure of the bins are characterized with the following parameters:
 
-* `milliseconds_from_event_to_first_bin`: The time in milliseconds from the event to the first bin. A negative value indicates that the first bin is before the event whereas a positive value indicates that the first bin is after the event. 
+* `milliseconds_from_event_to_first_bin`: The time in milliseconds from the event to the beginning of the first bin. A negative value indicates that the first bin is before the event whereas a positive value indicates that the first bin is after the event. 
 * `bin_width_in_milliseconds`: The width of each bin in milliseconds.
 
 
@@ -78,7 +78,7 @@ Note that in the diagram above, the `milliseconds_from_event_to_first_bin` is ne
 
 
 
-The `data` argument passed to the `BinnedAlignedSpikes` stores counts across all the event timestamps for each of the units. The data is a 3D array where the first dimension indexes the units, the second dimension indexes the event timestamps, and the third dimension indexes the bins where the counts are stored. The shape of the data is  `(number_of_units`, `number_of_event_repetitions`, `number_of_bins`). 
+The `data` argument passed to the `BinnedAlignedSpikes` stores counts across all the event timestamps for each of the units. The data is a 3D array where the first dimension indexes the units, the second dimension indexes the event timestamps, and the third dimension indexes the bins where the counts are stored. The shape of the data is  `(number_of_units`, `number_of_events`, `number_of_bins`). 
 
 
 The `event_timestamps` is used to store the timestamps of the events and should have the same length as the second dimension of `data`.

spec/ndx-binned-spikes.extensions.yaml

@@ -5,15 +5,23 @@ groups:
   doc: A data interface for binned spike data aligned to an event (e.g. a stimuli
     or the beginning of a trial).
   attributes:
+  - name: name
+    dtype: text
+    value: BinnedAlignedSpikes
+    doc: The name of this container
+  - name: description
+    dtype: text
+    value: Spikes data binned and aligned to event timestamps.
+    doc: A description of what the data represents
   - name: bin_width_in_milliseconds
     dtype: float64
     doc: The length in milliseconds of the bins
   - name: milliseconds_from_event_to_first_bin
     dtype: float64
     default_value: 0.0
-    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.
+    doc: The time in milliseconds from the event to the beginning of the first bin.
+      A negative value indicatesthat the first bin is before the event whereas a positive
+      value indicates that the first bin is after the event.
     required: false
   - name: units
     dtype:
@@ -26,17 +34,19 @@ groups:
     dtype: numeric
     dims:
     - - num_units
-      - number_of_event_repetitions
+      - number_of_events
       - number_of_bins
     shape:
     - - null
       - null
       - null
-    doc: TODO
+    doc: The binned data. It should be an array whose first dimension is the number
+      of units, the second dimension is the number of events, and the third dimension
+      is the number of bins.
   - name: event_timestamps
     dtype: float64
     dims:
-    - number_of_event_repetitions
+    - number_of_events
     shape:
     - null
-    doc: The timestamps at which the event occurred.
+    doc: The timestamps at which the events occurred.

src/pynwb/ndx_binned_spikes/__init__.py

@@ -3,7 +3,7 @@
 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
+from hdmf.utils import docval
 
 try:
     from importlib.resources import files
@@ -25,10 +25,11 @@
 # BinnedAlignedSpikes = get_class("BinnedAlignedSpikes", "ndx-binned-spikes")
 
 
-@register_class(neurodata_type="BinnedAlignedSpikes", namespace="ndx-binned-spikes") #noqa
+@register_class(neurodata_type="BinnedAlignedSpikes", namespace="ndx-binned-spikes")  # noqa
 class BinnedAlignedSpikes(NWBDataInterface):
     __nwbfields__ = (
         "name",
+        "description",
         "bin_width_in_milliseconds",
         "milliseconds_from_event_to_first_bin",
         "data",
@@ -37,6 +38,7 @@ class BinnedAlignedSpikes(NWBDataInterface):
     )
 
     DEFAULT_NAME = "BinnedAlignedSpikes"
+    DEFAULT_DESCRIPTION = "Spikes data binned and aligned to event timestamps."
 
     @docval(
         {
@@ -45,6 +47,12 @@ class BinnedAlignedSpikes(NWBDataInterface):
             "doc": "The name of this container",
             "default": DEFAULT_NAME,
         },
+        {
+            "name": "description",
+            "type": str,
+            "doc": "A description of what the data represents",
+            "default": DEFAULT_DESCRIPTION,
+        },
         {
             "name": "bin_width_in_milliseconds",
             "type": float,
@@ -54,52 +62,51 @@ class BinnedAlignedSpikes(NWBDataInterface):
             "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."
+                "The time in milliseconds from the event to the beginning of the first bin. A negative value indicates"
+                "that the first bin is before the event whereas a positive value indicates that the first bin is "
+                "after the event." 
             ),
             "default": 0.0,
         },
         {
             "name": "data",
             "type": "array_data",
             "shape": [(None, None, None)],
-            "doc": "The source of the data",
+            "doc": (
+                "The binned data. It should be an array whose first dimension is the number of units, "
+                "the second dimension is the number of events, and the third dimension is the number of bins."
+            ),
         },
         {
             "name": "event_timestamps",
             "type": "array_data",
-            "doc": "The timestamps at which the event occurred.",
+            "doc": "The timestamps at which the events occurred.",
             "shape": (None,),
         },
         {
             "name": "units",
-            "type": ("DynamicTableRegion"),
+            "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")
-        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"]
-        event_timestamps = args_to_process["event_timestamps"]
+        data = kwargs["data"]
+        event_timestamps = kwargs["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
+        super().__init__(name=kwargs["name"])
+
+        name = kwargs.pop("name")
+        super().__init__(name=name)
+
+        for key in kwargs:
+            setattr(self, key, kwargs[key])
+
+
 
 
 # Remove these functions from the package

src/pynwb/ndx_binned_spikes/testing/mock.py

@@ -6,7 +6,7 @@
 
 def mock_BinnedAlignedSpikes(
     number_of_units: int = 2,
-    number_of_event_repetitions: int = 4,
+    number_of_events: int = 4,
     number_of_bins: int = 3,
     bin_width_in_milliseconds: float = 20.0,
     milliseconds_from_event_to_first_bin: float = 1.0,
@@ -21,7 +21,7 @@ def mock_BinnedAlignedSpikes(
     ----------
     number_of_units : int, optional
         The number of different units (channels, neurons, etc.) to simulate.
-    number_of_event_repetitions : int, optional
+    number_of_events : int, optional
         The number of times an event is repeated.
     number_of_bins : int, optional
         The number of bins.
@@ -33,9 +33,9 @@ def mock_BinnedAlignedSpikes(
         Seed for the random number generator to ensure reproducibility.
     event_timestamps : np.ndarray, optional
         An array of timestamps for each event. If not provided, it will be automatically generated.
-        It should have size `number_of_event_repetitions`.
+        It should have size `number_of_events`.
     data : np.ndarray, optional
-        A 3D array of shape (number_of_units, number_of_event_repetitions, number_of_bins) representing
+        A 3D array of shape (number_of_units, number_of_events, number_of_bins) representing
         the binned spike data. If provided, it overrides the generation of mock data based on other parameters.
         Its shape should match the expected number of units, event repetitions, and bins.
 
@@ -62,21 +62,21 @@ def mock_BinnedAlignedSpikes(
     """
 
     if data is not None:
-        number_of_units, number_of_event_repetitions, number_of_bins = data.shape
+        number_of_units, number_of_events, number_of_bins = data.shape
     else:
         rng = np.random.default_rng(seed=seed)
-        data = rng.integers(low=0, high=100, size=(number_of_units, number_of_event_repetitions, number_of_bins))
+        data = rng.integers(low=0, high=100, size=(number_of_units, number_of_events, number_of_bins))
 
     if event_timestamps is None:
-        event_timestamps = np.arange(number_of_event_repetitions, dtype="float64")
+        event_timestamps = np.arange(number_of_events, dtype="float64")
     else:
         assert (
-            event_timestamps.shape[0] == number_of_event_repetitions
-        ), "The shape of `event_timestamps` does not match `number_of_event_repetitions`."
+            event_timestamps.shape[0] == number_of_events
+        ), "The shape of `event_timestamps` does not match `number_of_events`."
         event_timestamps = np.array(event_timestamps, dtype="float64")
 
     if event_timestamps.shape[0] != data.shape[1]:
-        raise ValueError("The shape of `event_timestamps` does not match `number_of_event_repetitions`.")
+        raise ValueError("The shape of `event_timestamps` does not match `number_of_events`.")
 
     binned_aligned_spikes = BinnedAlignedSpikes(
         bin_width_in_milliseconds=bin_width_in_milliseconds,

src/pynwb/tests/test_binned_aligned_spikes.py

@@ -21,7 +21,7 @@ def setUp(self):
 
         self.number_of_units = 2
         self.number_of_bins = 3
-        self.number_of_event_repetitions = 4
+        self.number_of_events = 4
         self.bin_width_in_milliseconds = 20.0
         self.milliseconds_from_event_to_first_bin = -100.0
         self.rng = np.random.default_rng(seed=0)
@@ -31,12 +31,12 @@ def setUp(self):
             high=100,
             size=(
                 self.number_of_units,
-                self.number_of_event_repetitions,
+                self.number_of_events,
                 self.number_of_bins,
             ),
         )
 
-        self.event_timestamps = np.arange(self.number_of_event_repetitions, dtype="float64")
+        self.event_timestamps = np.arange(self.number_of_events, dtype="float64")
 
         self.nwbfile = mock_NWBFile()
 
@@ -58,7 +58,7 @@ def test_constructor(self):
         )
 
         self.assertEqual(binned_aligned_spikes.data.shape[0], self.number_of_units)
-        self.assertEqual(binned_aligned_spikes.data.shape[1], self.number_of_event_repetitions)
+        self.assertEqual(binned_aligned_spikes.data.shape[1], self.number_of_events)
         self.assertEqual(binned_aligned_spikes.data.shape[2], self.number_of_bins)
 
     def test_constructor_units_region(self):

src/spec/create_extension_spec.py

@@ -27,18 +27,21 @@ def main():
 
     binned_aligned_spikes_data = NWBDatasetSpec(
         name="data",
-        doc="TODO",
+        doc=(
+            "The binned data. It should be an array whose first dimension is the number of units, the second dimension "
+            "is the number of events, and the third dimension is the number of bins."
+            ),
         dtype="numeric",  # TODO should this be a uint64?
         shape=[(None, None, None)],
-        dims=[("num_units", "number_of_event_repetitions", "number_of_bins")],
+        dims=[("num_units", "number_of_events", "number_of_bins")],
     )
 
     event_timestamps = NWBDatasetSpec(
         name="event_timestamps",
-        doc="The timestamps at which the event occurred.",
+        doc="The timestamps at which the events occurred.",
         dtype="float64",
         shape=(None,),
-        dims=("number_of_event_repetitions",),
+        dims=("number_of_events",),
     )
 
     binned_aligned_spikes = NWBGroupSpec(
@@ -48,6 +51,18 @@ def main():
         doc="A data interface for binned spike data aligned to an event (e.g. a stimuli or the beginning of a trial).",
         datasets=[binned_aligned_spikes_data, event_timestamps],
         attributes=[
+            NWBAttributeSpec(
+                name="name",
+                doc="The name of this container",
+                dtype="text",
+                value="BinnedAlignedSpikes",
+            ),
+            NWBAttributeSpec(
+                name="description",
+                doc="A description of what the data represents",
+                dtype="text",
+                value="Spikes data binned and aligned to event timestamps.",
+            ),
             NWBAttributeSpec(
                 name="bin_width_in_milliseconds",
                 doc="The length in milliseconds of the bins",
@@ -56,9 +71,10 @@ def main():
             NWBAttributeSpec(
                 name="milliseconds_from_event_to_first_bin",
                 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."
-                ),
+                "The time in milliseconds from the event to the beginning of the first bin. A negative value indicates"
+                "that the first bin is before the event whereas a positive value indicates that the first bin is "
+                "after the event." 
+            ),
                 dtype="float64",
                 default_value=0.0,
             ),