diff --git a/CHANGELOG.md b/CHANGELOG.md index 0354cf10..96e03b2a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ # v0.7.3 (Upcoming) ### New Checks +* Added `check_spike_times_without_nans` to detect NaN values in Units spike times, which indicate conversion bugs or unclean array padding. [#689](https://github.com/NeurodataWithoutBorders/nwbinspector/pull/689) ### Improvements * Raised the minimum required PyNWB to `>=4.0` to track the latest PyNWB release. [#708](https://github.com/NeurodataWithoutBorders/nwbinspector/pull/708) diff --git a/docs/best_practices/ecephys.rst b/docs/best_practices/ecephys.rst index 9af1e9cf..1a3989e3 100644 --- a/docs/best_practices/ecephys.rst +++ b/docs/best_practices/ecephys.rst @@ -151,6 +151,19 @@ Check function: :py:meth:`~nwbinspector.checks._ecephys.check_spike_times_not_in +.. _best_practice_spike_times_without_nans: + +No NaN Spike Times +~~~~~~~~~~~~~~~~~~ + +Spike times are physical event timestamps and must always be valid floating-point numbers. A NaN value in the +``spike_times`` column typically indicates a conversion bug or array padding that was not cleaned up before writing +to NWB. Unlike missing data in continuous signals, a spike either occurred at a specific time or it did not, so +NaN is never meaningful. Clean your spike time arrays to remove any NaN values before adding them to the Units table. + +Check function: :py:meth:`~nwbinspector.checks._ecephys.check_spike_times_without_nans` + + .. _best_practice_ascending_spike_times: Ascending Spike Times diff --git a/src/nwbinspector/checks/__init__.py b/src/nwbinspector/checks/__init__.py index c01ad2ab..2bf3777a 100644 --- a/src/nwbinspector/checks/__init__.py +++ b/src/nwbinspector/checks/__init__.py @@ -14,6 +14,7 @@ check_negative_spike_times, check_spike_times_not_in_samples, check_spike_times_not_in_unobserved_interval, + check_spike_times_without_nans, check_units_resolution_is_set, check_units_resolution_is_valid, check_units_table_duration, @@ -186,6 +187,7 @@ "check_spatial_series_degrees_magnitude", "check_spatial_series_unit", "check_ascending_spike_times", + "check_spike_times_without_nans", "check_electrical_series_unscaled_data", "check_units_resolution_is_valid", "check_units_resolution_is_set", diff --git a/src/nwbinspector/checks/_ecephys.py b/src/nwbinspector/checks/_ecephys.py index dd28edc5..da5eaf6b 100644 --- a/src/nwbinspector/checks/_ecephys.py +++ b/src/nwbinspector/checks/_ecephys.py @@ -244,6 +244,23 @@ def check_spike_times_not_in_unobserved_interval(units_table: Units, nunits: int return None +@register_check(importance=Importance.CRITICAL, neurodata_type=Units) +def check_spike_times_without_nans(units_table: Units) -> Optional[InspectorMessage]: + """ + Check if the Units table contains NaN values in spike times. + + Best Practice: :ref:`best_practice_spike_times_without_nans` + """ + if "spike_times" not in units_table: + return None + + if np.any(np.isnan(np.asarray(units_table["spike_times"].target.data[:]))): + return InspectorMessage( + message="Units table contains NaN spike times. Spike times should be valid timestamps in seconds." + ) + return None + + @register_check(importance=Importance.CRITICAL, neurodata_type=Units) def check_ascending_spike_times(units_table: Units, nelems: Optional[int] = NELEMS) -> Optional[InspectorMessage]: """ diff --git a/tests/unit_tests/test_ecephys.py b/tests/unit_tests/test_ecephys.py index f331ed9c..c2bf4037 100644 --- a/tests/unit_tests/test_ecephys.py +++ b/tests/unit_tests/test_ecephys.py @@ -20,6 +20,7 @@ check_negative_spike_times, check_spike_times_not_in_samples, check_spike_times_not_in_unobserved_interval, + check_spike_times_without_nans, check_units_resolution_is_set, check_units_resolution_is_valid, check_units_table_duration, @@ -437,6 +438,30 @@ def test_check_spike_times_not_in_unobserved_interval_multiple_units(): ) +class TestCheckSpikeTimesWithoutNans(TestCase): + def setUp(self): + self.units_table = Units() + + def test_spike_times_without_nans_valid(self): + self.units_table.add_unit(spike_times=[0.0, 0.1, 0.2]) + self.units_table.add_unit(spike_times=[1.0, 1.1, 1.2]) + assert check_spike_times_without_nans(units_table=self.units_table) is None + + def test_spike_times_with_nans(self): + self.units_table.add_unit(spike_times=[0.0, np.nan, 0.2]) + assert check_spike_times_without_nans(units_table=self.units_table) == InspectorMessage( + message="Units table contains NaN spike times. Spike times should be valid timestamps in seconds.", + importance=Importance.CRITICAL, + check_function_name="check_spike_times_without_nans", + object_type="Units", + object_name="Units", + location="/", + ) + + def test_spike_times_without_nans_empty(self): + assert check_spike_times_without_nans(units_table=self.units_table) is None + + class TestCheckAscendingSpikeTimes(TestCase): def setUp(self): self.units_table = Units()