Improve negative timestamps documentation and error (#545)

Add negative timestamps docs --------- Co-authored-by: Steph Prince <[email protected]>
NeurodataWithoutBorders · Dec 19, 2024 · e6ea845 · e6ea845
1 parent aca53f4
commit e6ea845
Show file tree

Hide file tree

Showing 5 changed files with 29 additions and 12 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,6 +1,9 @@
 # Upcoming
 
 
+## Improvements
+* Added a section for describing the issues with negative timestamps in `TimeSeries` [#545](https://github.com/NeurodataWithoutBorders/nwbinspector/pull/545)
+
 # v0.6.1
 
 ### Improvements
@@ -9,7 +12,6 @@
 ### Fixes
 * Fixed issue where the description check failed if the description was a list. [#535](https://github.com/NeurodataWithoutBorders/nwbinspector/pull/535)
 
-
 # v0.6.0
 
 ### Deprecation

diff --git a/docs/best_practices/nwbfile_metadata.rst b/docs/best_practices/nwbfile_metadata.rst
@@ -32,7 +32,6 @@ best guess. If the exact start time is unknown, then it is fine to simply set it
 
 Check functions: :py:meth:`~nwbinspector.checks._nwbfile_metadata.check_session_start_time_old_date`,
 :py:meth:`~nwbinspector.checks._nwbfile_metadata.check_session_start_time_future_date`,
-:py:meth:`~nwbinspector.checks._time_series.check_timestamp_of_the_first_sample_is_not_negative`
 :py:meth:`~nwbinspector.checks._tables.check_table_time_columns_are_not_negative`
 
 

diff --git a/docs/best_practices/time_series.rst b/docs/best_practices/time_series.rst
@@ -123,6 +123,21 @@ For segmented data, refer to the section covering :ref:`best_practice_timestamps
 Check function: :py:meth:`~nwbinspector.checks._time_series.check_regular_timestamps`
 
 
+.. _best_practice_avoid_negative_timestamps:
+
+Avoid Negative Timestamps
+~~~~~~~~~~~~~~~~~~~~~~~~~
+
+When writing :ref:`nwb-schema:sec-TimeSeries` data, avoid using negative timestamps.
+
+All timestamps in the NWBFile are written with respect to a global reference point (either ``timestamps_reference_time`` or ``session_start_time``).
+
+While negative timestamps are technically valid, they might introduce unnecessary complications for future users of the file.
+In most cases, negative timestamps may be indicative of an alignment error or a problem with the source data.
+As much as possible, re-align the session start time so that all timestamps are positive and correctly referenced to either ``timestamps_reference_time`` or ``session_start_time``.
+See :ref:`best_practice_global_time_reference` for more details.
+
+Check function: :py:meth:`~nwbinspector.checks._time_series.check_timestamp_of_the_first_sample_is_not_negative`
 
 .. _best_practice_chunk_data:
 

diff --git a/src/nwbinspector/checks/_time_series.py b/src/nwbinspector/checks/_time_series.py
@@ -125,15 +125,16 @@ def check_timestamp_of_the_first_sample_is_not_negative(time_series: TimeSeries)
     """
     Check that the timestamp of the first sample is not negative.
 
-    Best Practice: :ref:`best_practice_global_time_reference`
+    Best Practice: :ref:`best_practice_avoid_negative_timestamps`
     """
 
     first_timestamp = time_series.starting_time if time_series.starting_time is not None else time_series.timestamps[0]
     if first_timestamp < 0:
-        return InspectorMessage(
-            message="Timestamps should not be negative."
-            " It is recommended to align the `session_start_time` or `timestamps_reference_time` to be the earliest time value that occurs in the data, and shift all other signals accordingly."
+        message = (
+            "Timestamps should not be negative. This usually indicates a temporal misalignment of the data. "
+            "It is recommended to align the `session_start_time` or `timestamps_reference_time` to be the earliest time value that occurs in the data, and shift all other signals accordingly."
         )
+        return InspectorMessage(message=message)
 
     return None
 

diff --git a/tests/unit_tests/test_time_series.py b/tests/unit_tests/test_time_series.py
@@ -277,9 +277,8 @@ def test_check_timestamps_without_nans_fail():
 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 = (
-        "Timestamps should not be negative."
-        " It is recommended to align the `session_start_time` or `timestamps_reference_time` "
-        "to be the earliest time value that occurs in the data, and shift all other signals accordingly."
+        "Timestamps should not be negative. This usually indicates a temporal misalignment of the data. "
+        "It is recommended to align the `session_start_time` or `timestamps_reference_time` to be the earliest time value that occurs in the data, and shift all other signals accordingly."
     )
     assert check_timestamp_of_the_first_sample_is_not_negative(time_series) == InspectorMessage(
         message=message,
@@ -297,14 +296,15 @@ def test_check_timestamp_of_the_first_sample_is_not_negative_with_timestamps_pas
 
 
 def test_check_timestamp_of_the_first_sample_is_not_negative_with_starting_time_fail():
+
     time_series = pynwb.TimeSeries(
         name="test_time_series", unit="test_units", data=[1, 2, 3], starting_time=-1.0, rate=30.0
     )
     message = (
-        "Timestamps should not be negative."
-        " It is recommended to align the `session_start_time` or `timestamps_reference_time` "
-        "to be the earliest time value that occurs in the data, and shift all other signals accordingly."
+        "Timestamps should not be negative. This usually indicates a temporal misalignment of the data. "
+        "It is recommended to align the `session_start_time` or `timestamps_reference_time` to be the earliest time value that occurs in the data, and shift all other signals accordingly."
     )
+
     assert check_timestamp_of_the_first_sample_is_not_negative(time_series) == InspectorMessage(
         message=message,
         importance=Importance.BEST_PRACTICE_SUGGESTION,