Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add check for missing timezone. #458

Draft
wants to merge 14 commits into
base: dev
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 0 additions & 23 deletions .github/workflows/codespell.yml
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also discovered that we added this to pre-commit at some point: https://github.com/NeurodataWithoutBorders/nwbinspector/pull/458/files#diff-63a9c44a44acf85fea213a857769990937107cf072831e1a26808cfde9d096b9L13

but didn't remove the GitHub action

This file was deleted.

3 changes: 1 addition & 2 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@ repos:
- id: black

- repo: https://github.com/codespell-project/codespell
# Configuration for codespell is in pyproject.toml
rev: v2.2.6
rev: v2.3.0
hooks:
- id: codespell
additional_dependencies:
Expand Down
7 changes: 6 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
# Upcoming
# Upcoming (0.5.0)


### New Checks

* Add: `check_session_start_time_contains_time_zone` [#458](https://github.com/NeurodataWithoutBorders/nwbinspector/pull/458).

# v0.4.35

Expand Down
13 changes: 9 additions & 4 deletions docs/best_practices/nwbfile_metadata.rst
Original file line number Diff line number Diff line change
Expand Up @@ -19,21 +19,26 @@ objects in the :ref:`nwb-schema:sec-NWBFile` is the ``timestamps_reference_time`
``session_start_time``, but when writing multiple NWBFiles that are all designed to align to the same time reference,
the ``timestamp_reference_time`` used across all of the NWBFiles may be set separately from the ``session_start_time``.

All time-related data in the NWBFile should be synchronized to the ``timestamps_reference_time`` so that future users
are able to understand the timing of all events contained within the NWBFile.
All time-related data in the ``NWBFile`` should be synchronized to the ``timestamps_reference_time`` so that future users
are able to understand the timing of all events contained within the ``NWBFile``.

The ``timestamps_reference_time`` should also be the earliest timestamp in the file, giving all other time references
a positive value relative to that. There should be no time references which are negative.

Given the importance of this field within an :ref:`nwb-schema:sec-NWBFile`, is it critical that it be set to a proper
value. Default values should generally not be used for this field. If the true date is unknown, use your
best guess. If the exact start time is unknown, then it is fine to simply set it to midnight on that date.
best guess. If working with human participants and such data is protected information, use the date 1900-01-01.

**New in PyNWB 2.7.0, released May 2, 2024:**

The ``session_start_time`` is no longer required to have a timezone offset. This information is now optional, but recommended. If the timezone offset is not provided, it should **not** be assumed to be UTC.


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`
:py:meth:`~nwbinspector.checks.tables.check_table_time_columns_are_not_negative`,
:py:meth:`~nwbinspector.checks.nwbfile_metadata.check_session_start_time_contains_time_zone`,



Expand Down
3 changes: 1 addition & 2 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@ extend-exclude = '''
'''

[tool.codespell]
# Ref: https://github.com/codespell-project/codespell#using-a-config-file
skip = '.git*,*.pdf,*.css'
check-hidden = true
# ignore-regex = ''
# ignore-words-list = ''
ignore-words-list = 'assertin'
31 changes: 29 additions & 2 deletions src/nwbinspector/checks/nwbfile_metadata.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
"""Check functions that examine general NWBFile metadata."""

import re
from datetime import datetime
from datetime import datetime, date

from isodate import parse_duration, Duration
from pynwb import NWBFile, ProcessingModule
Expand Down Expand Up @@ -36,6 +36,22 @@ def check_session_start_time_old_date(nwbfile: NWBFile):
)


@register_check(importance=Importance.BEST_PRACTICE_SUGGESTION, neurodata_type=NWBFile)
def check_session_start_time_contains_time_zone(nwbfile: NWBFile):
"""
Check if the session_start_time contains a time zone.

Best Practice: :ref:`best_practice_global_time_reference`
"""
session_start_time = nwbfile.session_start_time
if isinstance(session_start_time, date):
return
if session_start_time.tzinfo is None:
CodyCBakerPhD marked this conversation as resolved.
Show resolved Hide resolved
return InspectorMessage(
message=(f"The session_start_time ({session_start_time}) does not contain a time zone.")
)


@register_check(importance=Importance.CRITICAL, neurodata_type=NWBFile)
def check_session_start_time_future_date(nwbfile: NWBFile):
"""
Expand All @@ -44,8 +60,19 @@ def check_session_start_time_future_date(nwbfile: NWBFile):
Best Practice: :ref:`best_practice_global_time_reference`
"""
session_start_time = nwbfile.session_start_time

if isinstance(session_start_time, date):
current_date = date.today()
if session_start_time > current_date:
return InspectorMessage(
message=(
f"The session_start_time ({session_start_time}) is set to a future date. "
"Please ensure that the session_start_time is set to the correct date and time."
)
)

current_time = datetime.now()
if session_start_time.tzinfo is not None:
if isinstance(session_start_time, datetime) and session_start_time.tzinfo is not None:
current_time = current_time.astimezone()
if session_start_time >= current_time:
return InspectorMessage(
Expand Down
39 changes: 38 additions & 1 deletion tests/unit_tests/test_nwbfile_metadata.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
from uuid import uuid4
from datetime import datetime, timezone
from datetime import datetime, timezone, date

import pynwb
import pytest
from pynwb import NWBFile, ProcessingModule
from pynwb.file import Subject

Expand All @@ -23,6 +25,7 @@
check_processing_module_name,
check_session_start_time_old_date,
check_session_start_time_future_date,
check_session_start_time_contains_time_zone,
PROCESSING_MODULE_CONFIG,
)
from nwbinspector.tools import make_minimal_nwbfile
Expand Down Expand Up @@ -56,6 +59,40 @@ def test_check_session_start_time_future_date_pass():
assert check_session_start_time_future_date(nwbfile) is None


@pytest.mark.skipif(pynwb.__version__ < "2.7.0", reason="Feature not supported in pynwb < 2.7.0")
def test_check_session_start_time_contains_time_zone_pass():
nwbfile = NWBFile(
session_description="",
identifier=str(uuid4()),
session_start_time=datetime(2010, 1, 1, 0, 0, 0, 0, timezone.utc),
)
assert check_session_start_time_contains_time_zone(nwbfile) is None


@pytest.mark.skipif(pynwb.__version__ < "2.7.0", reason="Feature not supported in pynwb < 2.7.0")
def test_check_session_start_time_is_date_contains_time_zone_pass():
nwbfile = NWBFile(
session_description="",
identifier=str(uuid4()),
session_start_time=date(2010, 1, 1),
)
assert check_session_start_time_contains_time_zone(nwbfile) is None


@pytest.mark.skipif(pynwb.__version__ < "2.7.0", reason="Feature not supported in pynwb < 2.7.0")
def test_check_session_start_time_contains_time_zone_fail():
session_start_time = datetime(2010, 1, 1, 0, 0, 0, 0)
CodyCBakerPhD marked this conversation as resolved.
Show resolved Hide resolved
nwbfile = NWBFile(session_description="", identifier=str(uuid4()), session_start_time=session_start_time)
assert check_session_start_time_contains_time_zone(nwbfile) == InspectorMessage(
message=f"The session_start_time ({session_start_time}) does not contain a time zone.",
importance=Importance.BEST_PRACTICE_SUGGESTION,
check_function_name="check_session_start_time_contains_time_zone",
object_type="NWBFile",
object_name="root",
location="/",
)


def test_check_session_start_time_future_date_fail():
nwbfile = NWBFile(
session_description="",
Expand Down
Loading