[New Check]: Check timestamps ascending with nans (#480)

* add check for timestamps w nans

* update is_ascending_series to discard nans

* add tests

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* minor fix

* update CHANGELOG.md

* Update src/nwbinspector/checks/time_series.py

Co-authored-by: Cody Baker <[email protected]>

* add a section in the best practices document

* add nelems to check fun

* change check fun name to check_timestamps_without_nans

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Cody Baker <[email protected]>

CHANGELOG.md

@@ -1,5 +1,9 @@
 # Upcoming
 
+### Improvements
+
+* Update util function `is_ascending_series` to discard nan values and add `check_timestamps_without_nans` fun to check if timestamps contain NaN values [#476](https://github.com/NeurodataWithoutBorders/nwbinspector/issues/476)
+
 # v0.4.37
 
 ### Fixes

docs/best_practices/time_series.rst

@@ -86,6 +86,23 @@ Check function: :py:meth:`~nwbinspector.checks.time_series.check_timestamps_asce
 
 
 
+.. _best_practice_timestamps_without_nans:
+
+Timestamps without NaNs
+~~~~~~~~~~~~~~~~~~~~~~~
+
+The ``timestamps`` field of a :ref:`nwb-schema:sec-TimeSeries` should not contain ``NaN`` values, as this can lead to
+ambiguity in time references and potential issues in downstream analyses.
+
+Ensure that all timestamps are valid numerical values. If gaps in time need to be represented, consider segmenting the
+data into separate :ref:`nwb-schema:sec-TimeSeries` objects with appropriate ``starting_time`` or use the ``timestamps``
+vector to explicitly represent time gaps.
+
+Check function: :py:meth:`~nwbinspector.checks.time_series.check_timestamps_without_nans`
+
+
+
+
 .. _best_practice_regular_timestamps:
 
 Timestamps vs. Start & Rate

src/nwbinspector/checks/time_series.py

@@ -93,6 +93,13 @@ def check_timestamps_ascending(time_series: TimeSeries, nelems=200):
         return InspectorMessage(f"{time_series.name} timestamps are not ascending.")
 
 
+@register_check(importance=Importance.BEST_PRACTICE_VIOLATION, neurodata_type=TimeSeries)
+def check_timestamps_without_nans(time_series: TimeSeries, nelems=200):
+    """Check if there are NaN values in the timestamps array."""
+    if time_series.timestamps is not None and np.isnan(time_series.timestamps[:nelems]).any():
+        return InspectorMessage(message=f"{time_series.name} timestamps contain NaN values.")
+
+
 @register_check(importance=Importance.BEST_PRACTICE_SUGGESTION, neurodata_type=TimeSeries)
 def check_timestamp_of_the_first_sample_is_not_negative(time_series: TimeSeries):
     """

src/nwbinspector/utils.py

@@ -91,9 +91,16 @@ def is_regular_series(series: np.ndarray, tolerance_decimals: int = 9):
 def is_ascending_series(series: Union[h5py.Dataset, ArrayLike], nelems: Optional[int] = None):
     """General purpose function for determining if a series is monotonic increasing."""
     if isinstance(series, h5py.Dataset):
-        differences = np.diff(_cache_data_selection(data=series, selection=slice(nelems)))
+        data = _cache_data_selection(data=series, selection=slice(nelems))
     else:
-        differences = np.diff(series[:nelems])  # already in memory, no need to cache
+        data = series[:nelems]
+
+    # Remove NaN values from the series
+    data = np.array(data)
+    valid_data = data[~np.isnan(data)]
+
+    # Compute the differences between consecutive elements
+    differences = np.diff(valid_data)
 
     return np.all(differences >= 0)
 

tests/test_utils.py

@@ -17,6 +17,8 @@
 
 import pytest
 
+import numpy as np
+
 
 def test_format_byte_size():
     assert format_byte_size(byte_size=12345) == "12.35KB"
@@ -145,6 +147,7 @@ def test_calculate_number_of_cpu_negative_value(self):
 def test_is_ascending_series():
     assert is_ascending_series(series=[1, 1, 1])
     assert is_ascending_series(series=[1, 2, 3])
+    assert is_ascending_series(series=[1, np.nan, 3])
     assert not is_ascending_series(series=[1, 2, 1])
 
 

tests/unit_tests/test_time_series.py

@@ -12,6 +12,7 @@
     check_data_orientation,
     check_timestamps_match_first_dimension,
     check_timestamps_ascending,
+    check_timestamps_without_nans,
     check_missing_unit,
     check_resolution,
     check_timestamp_of_the_first_sample_is_not_negative,
@@ -227,6 +228,13 @@ def test_pass_check_timestamps_ascending_pass():
     assert check_timestamps_ascending(time_series) is None
 
 
+def test_pass_check_timestamps_ascending_with_nans_pass():
+    time_series = pynwb.TimeSeries(
+        name="test_time_series", unit="test_units", data=[1, 2, 3], timestamps=[1, np.nan, 3]
+    )
+    assert check_timestamps_ascending(time_series) is None
+
+
 def test_check_timestamps_ascending_fail():
     time_series = pynwb.TimeSeries(name="test_time_series", unit="test_units", data=[1, 2, 3], timestamps=[1, 3, 2])
     assert check_timestamps_ascending(time_series) == InspectorMessage(
@@ -239,6 +247,39 @@ def test_check_timestamps_ascending_fail():
     )
 
 
+def test_check_timestamps_ascending_with_nans_fail():
+    time_series = pynwb.TimeSeries(
+        name="test_time_series", unit="test_units", data=[1, 2, 3], timestamps=[np.nan, 3, 2]
+    )
+    assert check_timestamps_ascending(time_series) == InspectorMessage(
+        message="test_time_series timestamps are not ascending.",
+        importance=Importance.BEST_PRACTICE_VIOLATION,
+        check_function_name="check_timestamps_ascending",
+        object_type="TimeSeries",
+        object_name="test_time_series",
+        location="/",
+    )
+
+
+def test_check_timestamps_without_nans_pass():
+    time_series = pynwb.TimeSeries(name="test_time_series", unit="test_units", data=[1, 2, 3], timestamps=[1, 2, 3])
+    assert check_timestamps_without_nans(time_series) is None
+
+
+def test_check_timestamps_without_nans_fail():
+    time_series = pynwb.TimeSeries(
+        name="test_time_series", unit="test_units", data=[1, 2, 3], timestamps=[np.nan, 2, 3]
+    )
+    assert check_timestamps_without_nans(time_series) == InspectorMessage(
+        message="test_time_series timestamps contain NaN values.",
+        importance=Importance.BEST_PRACTICE_VIOLATION,
+        check_function_name="check_timestamps_without_nans",
+        object_type="TimeSeries",
+        object_name="test_time_series",
+        location="/",
+    )
+
+
 def test_check_timestamp_of_the_first_sample_is_not_negative_with_timestamps_fail():
     time_series = pynwb.TimeSeries(name="test_time_series", unit="test_units", data=[1, 2, 3], timestamps=[-1, 0, 1])
     message = (