diff --git a/CHANGELOG.md b/CHANGELOG.md index db78fc55..ac6c6170 100644 --- 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 index 53c5878c..cc8319b7 100644 --- 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 index f6b3d2ab..c3adf689 100644 --- 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 index 8956171f..f6050fae 100644 --- 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 index 429b8048..7ffa12e6 100644 --- 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,