From c136ed87aca0c1a29488e122dbabe788c385acf4 Mon Sep 17 00:00:00 2001 From: Alessandra Trapani Date: Mon, 18 Sep 2023 11:37:15 +0200 Subject: [PATCH 01/10] add check function for ascending spike times --- src/nwbinspector/checks/ecephys.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/nwbinspector/checks/ecephys.py b/src/nwbinspector/checks/ecephys.py index 5044bd8d1..d18f0ee89 100644 --- a/src/nwbinspector/checks/ecephys.py +++ b/src/nwbinspector/checks/ecephys.py @@ -79,3 +79,17 @@ def check_spike_times_not_in_unobserved_interval(units_table: Units, nunits: int "observed intervals." ) ) + + +@register_check(importance=Importance.BEST_PRACTICE_VIOLATION, neurodata_type=Units) +def check_ascending_spike_times(units_table: Units): + """Check that the values in the timestamps array are strictly increasing.""" + if "spike_times" not in units_table: + return + differences = np.diff(np.asarray(units_table["spike_times"].target.data[:])) + if np.all(differences >= 0): + return InspectorMessage( + message=( + "This Units table contains spike times that are not ascending." + ) + ) \ No newline at end of file From dff4675bff0ca9502e68068a0829df5a3aea00a2 Mon Sep 17 00:00:00 2001 From: Alessandra Trapani Date: Fri, 22 Sep 2023 15:24:33 +0200 Subject: [PATCH 02/10] fix iterate over the units --- src/nwbinspector/checks/ecephys.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/nwbinspector/checks/ecephys.py b/src/nwbinspector/checks/ecephys.py index d18f0ee89..2f95917a6 100644 --- a/src/nwbinspector/checks/ecephys.py +++ b/src/nwbinspector/checks/ecephys.py @@ -86,8 +86,9 @@ def check_ascending_spike_times(units_table: Units): """Check that the values in the timestamps array are strictly increasing.""" if "spike_times" not in units_table: return - differences = np.diff(np.asarray(units_table["spike_times"].target.data[:])) - if np.all(differences >= 0): + for spike_times_per_unit in units_table["spike_times"]: + differences_per_unit = np.diff(spike_times_per_unit) + if np.all(differences_per_unit >= 0): return InspectorMessage( message=( "This Units table contains spike times that are not ascending." From e075e073ea1ee2671b9b84b834cb3134ce2980e7 Mon Sep 17 00:00:00 2001 From: Alessandra Trapani Date: Fri, 22 Sep 2023 15:24:43 +0200 Subject: [PATCH 03/10] add unit test --- tests/unit_tests/test_ecephys.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/unit_tests/test_ecephys.py b/tests/unit_tests/test_ecephys.py index ccd3ab636..c60a50a45 100644 --- a/tests/unit_tests/test_ecephys.py +++ b/tests/unit_tests/test_ecephys.py @@ -15,6 +15,7 @@ check_electrical_series_dims, check_electrical_series_reference_electrodes_table, check_spike_times_not_in_unobserved_interval, + check_ascending_spike_times, ) @@ -215,3 +216,24 @@ def test_check_spike_times_not_in_unobserved_interval_multiple_units(): object_name="TestUnits", location="/", ) + +def test_check_ascending_spike_times_pass(): + units_table = Units() + units_table.add_unit(spike_times=[0.0, 0.1]) + units_table.add_unit(spike_times=[1.0, 2.0]) + assert check_ascending_spike_times(units_table=units_table) is None + +def test_check_ascending_spike_times_fail(): + units_table = Units(name="TestUnits") + units_table.add_unit(spike_times=[0.0, 0.1]) + units_table.add_unit(spike_times=[2.0, 1.0]) + assert check_ascending_spike_times(units_table=units_table) == InspectorMessage( + message=( + "This Units table contains spike times that are not ascending." + ), + importance=Importance.BEST_PRACTICE_VIOLATION, + check_function_name="check_ascending_spike_times", + object_type="Units", + object_name="TestUnits", + location="/", + ) \ No newline at end of file From fb064463e7c4908dd583961015026562b6f07577 Mon Sep 17 00:00:00 2001 From: Alessandra Trapani Date: Fri, 22 Sep 2023 15:26:30 +0200 Subject: [PATCH 04/10] formatted code --- src/nwbinspector/checks/ecephys.py | 6 +----- tests/unit_tests/test_ecephys.py | 10 +++++----- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/src/nwbinspector/checks/ecephys.py b/src/nwbinspector/checks/ecephys.py index 2f95917a6..00135bf44 100644 --- a/src/nwbinspector/checks/ecephys.py +++ b/src/nwbinspector/checks/ecephys.py @@ -89,8 +89,4 @@ def check_ascending_spike_times(units_table: Units): for spike_times_per_unit in units_table["spike_times"]: differences_per_unit = np.diff(spike_times_per_unit) if np.all(differences_per_unit >= 0): - return InspectorMessage( - message=( - "This Units table contains spike times that are not ascending." - ) - ) \ No newline at end of file + return InspectorMessage(message=("This Units table contains spike times that are not ascending.")) diff --git a/tests/unit_tests/test_ecephys.py b/tests/unit_tests/test_ecephys.py index c60a50a45..5fc1c3615 100644 --- a/tests/unit_tests/test_ecephys.py +++ b/tests/unit_tests/test_ecephys.py @@ -217,23 +217,23 @@ def test_check_spike_times_not_in_unobserved_interval_multiple_units(): location="/", ) + def test_check_ascending_spike_times_pass(): units_table = Units() units_table.add_unit(spike_times=[0.0, 0.1]) units_table.add_unit(spike_times=[1.0, 2.0]) assert check_ascending_spike_times(units_table=units_table) is None + def test_check_ascending_spike_times_fail(): units_table = Units(name="TestUnits") units_table.add_unit(spike_times=[0.0, 0.1]) units_table.add_unit(spike_times=[2.0, 1.0]) - assert check_ascending_spike_times(units_table=units_table) == InspectorMessage( - message=( - "This Units table contains spike times that are not ascending." - ), + assert check_ascending_spike_times(units_table=units_table) == InspectorMessage( + message=("This Units table contains spike times that are not ascending."), importance=Importance.BEST_PRACTICE_VIOLATION, check_function_name="check_ascending_spike_times", object_type="Units", object_name="TestUnits", location="/", - ) \ No newline at end of file + ) From 38a85079399317b2706f6f9b621911ed06558cc8 Mon Sep 17 00:00:00 2001 From: Alessandra Trapani <55453048+alessandratrapani@users.noreply.github.com> Date: Mon, 25 Sep 2023 17:13:25 +0200 Subject: [PATCH 05/10] index shifting strategy Co-authored-by: Cody Baker <51133164+CodyCBakerPhD@users.noreply.github.com> --- src/nwbinspector/checks/ecephys.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/nwbinspector/checks/ecephys.py b/src/nwbinspector/checks/ecephys.py index 00135bf44..803502a21 100644 --- a/src/nwbinspector/checks/ecephys.py +++ b/src/nwbinspector/checks/ecephys.py @@ -87,6 +87,5 @@ def check_ascending_spike_times(units_table: Units): if "spike_times" not in units_table: return for spike_times_per_unit in units_table["spike_times"]: - differences_per_unit = np.diff(spike_times_per_unit) - if np.all(differences_per_unit >= 0): - return InspectorMessage(message=("This Units table contains spike times that are not ascending.")) + if not np.all(spike_times_per_unit[:-1] <= spike_times_per_unit[1:]): + return InspectorMessage(message=("This Units table contains spike times that are not ascending.")) From e7d05855dd2e224931d0f5505d77574bdd40b4f0 Mon Sep 17 00:00:00 2001 From: Alessandra Trapani Date: Mon, 25 Sep 2023 17:19:23 +0200 Subject: [PATCH 06/10] add unit test missing spike times --- tests/unit_tests/test_ecephys.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/unit_tests/test_ecephys.py b/tests/unit_tests/test_ecephys.py index 5fc1c3615..34f8432d3 100644 --- a/tests/unit_tests/test_ecephys.py +++ b/tests/unit_tests/test_ecephys.py @@ -225,6 +225,13 @@ def test_check_ascending_spike_times_pass(): assert check_ascending_spike_times(units_table=units_table) is None +def test_check_ascending_spike_times_w_missing_spike_times_pass(): + units_table = Units() + units_table.add_unit(spike_times=[0.0, 0.1]) + units_table.add_unit(spike_times=[]) + assert check_ascending_spike_times(units_table=units_table) is None + + def test_check_ascending_spike_times_fail(): units_table = Units(name="TestUnits") units_table.add_unit(spike_times=[0.0, 0.1]) From 4a799a27fceec43729f017745c5ab7bf69120943 Mon Sep 17 00:00:00 2001 From: Alessandra Trapani Date: Tue, 26 Sep 2023 18:48:32 +0200 Subject: [PATCH 07/10] best_practice_ascending_spike_times in ecephys.rst --- docs/best_practices/ecephys.rst | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/docs/best_practices/ecephys.rst b/docs/best_practices/ecephys.rst index fffe593af..25a7615a7 100644 --- a/docs/best_practices/ecephys.rst +++ b/docs/best_practices/ecephys.rst @@ -83,3 +83,14 @@ Observation Intervals The ``obs_intervals`` field of the :ref:`nwb-schema:sec-units-src` table is used to indicate periods of time where the underlying electrical signal(s) were not observed. This can happen if the recording site moves away from the unit, or if the recording is stopped. Since the channel is not observed, it is not determinable whether a spike occurred during this time. Therefore, there should not be any identified spike times for units matched to those electrical signal(s) occurring outside of the defined ``obs_intervals``. If this variable is not set, it is assumed that all time is observed. Check function: :py:meth:`~nwbinspector.checks.ecephys.check_spike_times_not_in_unobserved_interval` + + + +.. _best_practice_ascending_spike_times: + +Ascending Spike Times +~~~~~~~~~~~~~~~~~~~~~ + +All spike times should be sorted in ascending order. Spike times may reset to zero about halfway through. This can happen if the spikes are not properly aligned with the ``timestamps_reference_time`` of the :ref:`nwb-schema:sec-NWBFile`. + +Check function: :py:meth:`~nwbinspector.checks.ecephys.check_ascending_spike_times` From 673cfb02abc432aab39b1bc1d5e4c7c53518bce4 Mon Sep 17 00:00:00 2001 From: Alessandra Trapani Date: Tue, 26 Sep 2023 18:52:22 +0200 Subject: [PATCH 08/10] add test for missing_column_skip --- tests/unit_tests/test_ecephys.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/unit_tests/test_ecephys.py b/tests/unit_tests/test_ecephys.py index 34f8432d3..6ef1ddd19 100644 --- a/tests/unit_tests/test_ecephys.py +++ b/tests/unit_tests/test_ecephys.py @@ -232,6 +232,11 @@ def test_check_ascending_spike_times_w_missing_spike_times_pass(): assert check_ascending_spike_times(units_table=units_table) is None +def test_check_ascending_spike_times_missing_column_skip(): + units_table = Units() + assert check_ascending_spike_times(units_table=units_table) is None + + def test_check_ascending_spike_times_fail(): units_table = Units(name="TestUnits") units_table.add_unit(spike_times=[0.0, 0.1]) From 4a44c272e11c32aaf3e28c4f4815023a76003f42 Mon Sep 17 00:00:00 2001 From: Alessandra Trapani <55453048+alessandratrapani@users.noreply.github.com> Date: Tue, 26 Sep 2023 19:51:40 +0200 Subject: [PATCH 09/10] Update docs/best_practices/ecephys.rst Co-authored-by: Ben Dichter --- docs/best_practices/ecephys.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/best_practices/ecephys.rst b/docs/best_practices/ecephys.rst index 25a7615a7..8dfbb5ad8 100644 --- a/docs/best_practices/ecephys.rst +++ b/docs/best_practices/ecephys.rst @@ -91,6 +91,6 @@ Check function: :py:meth:`~nwbinspector.checks.ecephys.check_spike_times_not_in_ Ascending Spike Times ~~~~~~~~~~~~~~~~~~~~~ -All spike times should be sorted in ascending order. Spike times may reset to zero about halfway through. This can happen if the spikes are not properly aligned with the ``timestamps_reference_time`` of the :ref:`nwb-schema:sec-NWBFile`. +All spike times should be sorted in ascending order. If they reset to zero, this can be a sign that spikes are not properly aligned with the ``timestamps_reference_time`` of the :ref:`nwb-schema:sec-NWBFile`. Check function: :py:meth:`~nwbinspector.checks.ecephys.check_ascending_spike_times` From 5111059444eece914742a4a29d231282e4d19e9e Mon Sep 17 00:00:00 2001 From: Alessandra Trapani Date: Fri, 29 Sep 2023 15:48:49 +0200 Subject: [PATCH 10/10] update CHANGELOG.md --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b407d934f..146bc8517 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Upcoming +### New Checks + +* Added `check_ascending_spike_times` for ensuring spike times are sorted in ascending order. [#394](https://github.com/NeurodataWithoutBorders/nwbinspector/issues/394) + # v0.4.30 ### Fixes