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 PyNWB 3.0 support #557

Open
wants to merge 4 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
6 changes: 4 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,15 +1,17 @@
# Upcoming
# v.0.6.2 (Upcoming)

### Deprecation
* Remove s3fs dependency, which was causing dependency management issues [#549](https://github.com/NeurodataWithoutBorders/nwbinspector/pull/549)

### Fixes
* Fix wrongly triggered compression check [#552](https://github.com/NeurodataWithoutBorders/nwbinspector/pull/552)

## Improvements
### Improvements
* Added a section for describing the issues with negative timestamps in `TimeSeries` [#545](https://github.com/NeurodataWithoutBorders/nwbinspector/pull/545)
* Use alternate way of generating `TimeSeries` objects to avoid new pynwb error when the shape of the first dimension of
data does not match the length of timestamps [#556](https://github.com/NeurodataWithoutBorders/nwbinspector/pull/556)
* Added support for PyNWB 3.0 [#557](https://github.com/NeurodataWithoutBorders/nwbinspector/pull/557)


# v0.6.1

Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ classifiers = [
]
requires-python = ">=3.9"
dependencies = [
"pynwb>=2.8,<3", # NWB Inspector should always be used with most recent minor versions of PyNWB
"pynwb>=2.8", # NWB Inspector should always be used with most recent minor versions of PyNWB
"hdmf-zarr",
"fsspec",
"requests",
Expand Down
8 changes: 7 additions & 1 deletion src/nwbinspector/_nwb_inspection.py
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,13 @@ def inspect_nwbfile(
in_memory_nwbfile = read_nwbfile(nwbfile_path=nwbfile_path)

if not skip_validate:
validation_errors, _ = pynwb.validate(paths=[nwbfile_path])
# TODO - update validation call when pynwb 3.0 is the minimal
validation_result = pynwb.validate(paths=[nwbfile_path])
if isinstance(validation_result, tuple):
validation_errors = validation_result[0]
else:
validation_errors = validation_result

for validation_error in validation_errors:
yield InspectorMessage(
message=validation_error.reason,
Expand Down
2 changes: 1 addition & 1 deletion src/nwbinspector/checks/_images.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,6 @@ def check_index_series_points_to_image(index_series: IndexSeries) -> Optional[In
return InspectorMessage(
message="Pointing an IndexSeries to a TimeSeries will be deprecated. Please point to an Images "
"container instead."
)
) # TODO - update when pynwb 3.0 is the minimum version

return None
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,20 @@ Found 5 issues over 2 files:
0 CRITICAL
===========

0.0 ./testing0.nwb.hdf5: check_data_orientation - 'SpatialSeries' object at location '/processing/behavior/Position/my_spatial_series'
0.0 ./testing0.hdf5.nwb: check_data_orientation - 'SpatialSeries' object at location '/processing/behavior/Position/my_spatial_series'
Message: Data may be in the wrong orientation. Time should be in the first dimension, and is usually the longest dimension. Here, another dimension is longer.

0.1 ./testing0.nwb.hdf5: check_timestamps_match_first_dimension - 'TimeSeries' object at location '/acquisition/test_time_series_3'
0.1 ./testing0.hdf5.nwb: check_timestamps_match_first_dimension - 'TimeSeries' object at location '/acquisition/test_time_series_3'
Message: The length of the first dimension of data (4) does not match the length of timestamps (3).

1 BEST_PRACTICE_VIOLATION
==========================

1.2 ./testing0.nwb.hdf5 and 1 other file: check_regular_timestamps - 'TimeSeries' object at location '/acquisition/test_time_series_2'
1.2 ./testing0.hdf5.nwb and 1 other file: check_regular_timestamps - 'TimeSeries' object at location '/acquisition/test_time_series_2'
Message: TimeSeries appears to have a constant sampling rate. Consider specifying starting_time=1.2 and rate=0.5 instead of timestamps.

2 BEST_PRACTICE_SUGGESTION
===========================

2.3 ./testing0.nwb.hdf5: check_small_dataset_compression - 'TimeSeries' object at location '/acquisition/test_time_series_1'
2.3 ./testing0.hdf5.nwb: check_small_dataset_compression - 'TimeSeries' object at location '/acquisition/test_time_series_1'
Message: data is not compressed. Consider enabling compression when writing a dataset.
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,20 @@ Found 5 issues over 2 files:
0 CRITICAL
===========

0.0 ./testing0.nwb.zarr: check_data_orientation - 'SpatialSeries' object at location '/processing/behavior/Position/my_spatial_series'
0.0 ./testing0.zarr.nwb: check_data_orientation - 'SpatialSeries' object at location '/processing/behavior/Position/my_spatial_series'
Message: Data may be in the wrong orientation. Time should be in the first dimension, and is usually the longest dimension. Here, another dimension is longer.

0.1 ./testing0.nwb.zarr: check_timestamps_match_first_dimension - 'TimeSeries' object at location '/acquisition/test_time_series_3'
0.1 ./testing0.zarr.nwb: check_timestamps_match_first_dimension - 'TimeSeries' object at location '/acquisition/test_time_series_3'
Message: The length of the first dimension of data (4) does not match the length of timestamps (3).

1 BEST_PRACTICE_VIOLATION
==========================

1.2 ./testing0.nwb.zarr and 1 other file: check_regular_timestamps - 'TimeSeries' object at location '/acquisition/test_time_series_2'
1.2 ./testing0.zarr.nwb and 1 other file: check_regular_timestamps - 'TimeSeries' object at location '/acquisition/test_time_series_2'
Message: TimeSeries appears to have a constant sampling rate. Consider specifying starting_time=1.2 and rate=0.5 instead of timestamps.

2 BEST_PRACTICE_SUGGESTION
===========================

2.3 ./testing0.nwb.zarr: check_small_dataset_compression - 'TimeSeries' object at location '/acquisition/test_time_series_1'
2.3 ./testing0.zarr.nwb: check_small_dataset_compression - 'TimeSeries' object at location '/acquisition/test_time_series_1'
Message: data is not compressed. Consider enabling compression when writing a dataset.
Original file line number Diff line number Diff line change
Expand Up @@ -14,32 +14,32 @@ Found 10 issues over 2 files:
0 CRITICAL
===========

0.0 ./testing0.nwb.hdf5 and 1 other file: check_subject_exists - 'NWBFile' object at location '/'
0.0 ./testing0.hdf5.nwb and 1 other file: check_subject_exists - 'NWBFile' object at location '/'
Message: Subject is missing.

0.1 ./testing0.nwb.hdf5: check_timestamps_match_first_dimension - 'TimeSeries' object at location '/acquisition/test_time_series_3'
0.1 ./testing0.hdf5.nwb: check_timestamps_match_first_dimension - 'TimeSeries' object at location '/acquisition/test_time_series_3'
Message: The length of the first dimension of data (4) does not match the length of timestamps (3).

1 BEST_PRACTICE_VIOLATION
==========================

1.2 ./testing0.nwb.hdf5: check_time_interval_time_columns - 'TimeIntervals' object with name 'test_table'
1.2 ./testing0.hdf5.nwb: check_time_interval_time_columns - 'TimeIntervals' object with name 'test_table'
Message: ['start_time'] are time columns but the values are not in ascending order. All times should be in seconds with respect to the session start time.

1.3 ./testing0.nwb.hdf5: check_regular_timestamps - 'TimeSeries' object at location '/acquisition/test_time_series_2'
1.3 ./testing0.hdf5.nwb: check_regular_timestamps - 'TimeSeries' object at location '/acquisition/test_time_series_2'
Message: TimeSeries appears to have a constant sampling rate. Consider specifying starting_time=1.2 and rate=0.5 instead of timestamps.

1.4 ./testing0.nwb.hdf5: check_data_orientation - 'SpatialSeries' object at location '/processing/behavior/Position/my_spatial_series'
1.4 ./testing0.hdf5.nwb: check_data_orientation - 'SpatialSeries' object at location '/processing/behavior/Position/my_spatial_series'
Message: Data may be in the wrong orientation. Time should be in the first dimension, and is usually the longest dimension. Here, another dimension is longer.

1.5 ./testing0.nwb.hdf5: check_missing_unit - 'TimeSeries' object at location '/acquisition/test_time_series_3'
1.5 ./testing0.hdf5.nwb: check_missing_unit - 'TimeSeries' object at location '/acquisition/test_time_series_3'
Message: Missing text for attribute 'unit'. Please specify the scientific unit of the 'data'.

1.6 ./testing0.nwb.hdf5: check_missing_unit - 'TimeSeries' object at location '/acquisition/test_time_series_2'
1.6 ./testing0.hdf5.nwb: check_missing_unit - 'TimeSeries' object at location '/acquisition/test_time_series_2'
Message: Missing text for attribute 'unit'. Please specify the scientific unit of the 'data'.

1.7 ./testing0.nwb.hdf5: check_missing_unit - 'TimeSeries' object at location '/acquisition/test_time_series_1'
1.7 ./testing0.hdf5.nwb: check_missing_unit - 'TimeSeries' object at location '/acquisition/test_time_series_1'
Message: Missing text for attribute 'unit'. Please specify the scientific unit of the 'data'.

1.8 ./testing1.nwb.hdf5: check_data_orientation - 'TimeSeries' object at location '/acquisition/my_spatial_series'
1.8 ./testing1.hdf5.nwb: check_data_orientation - 'TimeSeries' object at location '/acquisition/my_spatial_series'
Message: Data may be in the wrong orientation. Time should be in the first dimension, and is usually the longest dimension. Here, another dimension is longer.
Original file line number Diff line number Diff line change
Expand Up @@ -14,32 +14,32 @@ Found 10 issues over 2 files:
0 CRITICAL
===========

0.0 ./testing0.nwb.zarr and 1 other file: check_subject_exists - 'NWBFile' object at location '/'
0.0 ./testing0.zarr.nwb and 1 other file: check_subject_exists - 'NWBFile' object at location '/'
Message: Subject is missing.

0.1 ./testing0.nwb.zarr: check_timestamps_match_first_dimension - 'TimeSeries' object at location '/acquisition/test_time_series_3'
0.1 ./testing0.zarr.nwb: check_timestamps_match_first_dimension - 'TimeSeries' object at location '/acquisition/test_time_series_3'
Message: The length of the first dimension of data (4) does not match the length of timestamps (3).

1 BEST_PRACTICE_VIOLATION
==========================

1.2 ./testing0.nwb.zarr: check_time_interval_time_columns - 'TimeIntervals' object with name 'test_table'
1.2 ./testing0.zarr.nwb: check_time_interval_time_columns - 'TimeIntervals' object with name 'test_table'
Message: ['start_time'] are time columns but the values are not in ascending order. All times should be in seconds with respect to the session start time.

1.3 ./testing0.nwb.zarr: check_regular_timestamps - 'TimeSeries' object at location '/acquisition/test_time_series_2'
1.3 ./testing0.zarr.nwb: check_regular_timestamps - 'TimeSeries' object at location '/acquisition/test_time_series_2'
Message: TimeSeries appears to have a constant sampling rate. Consider specifying starting_time=1.2 and rate=0.5 instead of timestamps.

1.4 ./testing0.nwb.zarr: check_data_orientation - 'SpatialSeries' object at location '/processing/behavior/Position/my_spatial_series'
1.4 ./testing0.zarr.nwb: check_data_orientation - 'SpatialSeries' object at location '/processing/behavior/Position/my_spatial_series'
Message: Data may be in the wrong orientation. Time should be in the first dimension, and is usually the longest dimension. Here, another dimension is longer.

1.5 ./testing0.nwb.zarr: check_missing_unit - 'TimeSeries' object at location '/acquisition/test_time_series_3'
1.5 ./testing0.zarr.nwb: check_missing_unit - 'TimeSeries' object at location '/acquisition/test_time_series_3'
Message: Missing text for attribute 'unit'. Please specify the scientific unit of the 'data'.

1.6 ./testing0.nwb.zarr: check_missing_unit - 'TimeSeries' object at location '/acquisition/test_time_series_2'
1.6 ./testing0.zarr.nwb: check_missing_unit - 'TimeSeries' object at location '/acquisition/test_time_series_2'
Message: Missing text for attribute 'unit'. Please specify the scientific unit of the 'data'.

1.7 ./testing0.nwb.zarr: check_missing_unit - 'TimeSeries' object at location '/acquisition/test_time_series_1'
1.7 ./testing0.zarr.nwb: check_missing_unit - 'TimeSeries' object at location '/acquisition/test_time_series_1'
Message: Missing text for attribute 'unit'. Please specify the scientific unit of the 'data'.

1.8 ./testing1.nwb.zarr: check_data_orientation - 'TimeSeries' object at location '/acquisition/my_spatial_series'
1.8 ./testing1.zarr.nwb: check_data_orientation - 'TimeSeries' object at location '/acquisition/my_spatial_series'
Message: Data may be in the wrong orientation. Time should be in the first dimension, and is usually the longest dimension. Here, another dimension is longer.
11 changes: 6 additions & 5 deletions tests/test_inspector.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,8 @@ def assertLogFileContentsEqual(
test_file_lines[line_number + 2] = "NWBInspector version: 0.3.6"
if ".nwb" in test_line:
# Transform temporary testing path and formatted to hardcoded fake path
str_loc = test_line.find(".nwb")
suffix = IO_CLASSES_TO_BACKEND[self.BackendIOClass]
str_loc = test_line.find(f".{suffix}.nwb")
correction_str = test_line.replace(test_line[5 : str_loc - 8], "./") # noqa: E203 (black)
test_file_lines[line_number] = correction_str
self.assertEqual(first=test_file_lines[skip_first_n_lines : -(1 + skip_last_n_lines)], second=true_file_lines)
Expand Down Expand Up @@ -193,7 +194,7 @@ def setUpClass(cls):
add_non_matching_timestamps_dimension(nwbfiles[3])

suffix = IO_CLASSES_TO_BACKEND[cls.BackendIOClass]
cls.nwbfile_paths = [str(cls.tempdir / f"testing{j}.nwb.{suffix}") for j in range(num_nwbfiles)]
cls.nwbfile_paths = [str(cls.tempdir / f"testing{j}.{suffix}.nwb") for j in range(num_nwbfiles)]
cls.nwbfile_paths[3] = str(cls.tempdir / "._testing3.nwb")
for nwbfile_path, nwbfile in zip(cls.nwbfile_paths, nwbfiles):
with cls.BackendIOClass(path=nwbfile_path, mode="w") as io:
Expand Down Expand Up @@ -686,7 +687,7 @@ def setUpClass(cls):
add_flipped_data_orientation_to_acquisition(nwbfiles[1])

suffix = IO_CLASSES_TO_BACKEND[cls.BackendIOClass]
cls.nwbfile_paths = [str(cls.tempdir / f"testing{j}.nwb.{suffix}") for j in range(num_nwbfiles)]
cls.nwbfile_paths = [str(cls.tempdir / f"testing{j}.{suffix}.nwb") for j in range(num_nwbfiles)]
for nwbfile_path, nwbfile in zip(cls.nwbfile_paths, nwbfiles):
with cls.BackendIOClass(path=nwbfile_path, mode="w") as io:
io.write(nwbfile)
Expand Down Expand Up @@ -800,7 +801,7 @@ def setUpClass(cls):

suffix = IO_CLASSES_TO_BACKEND[cls.BackendIOClass]
cls.unique_id_nwbfile_paths = [
str(cls.tempdir / f"unique_id_testing{j}.nwb.{suffix}") for j in range(num_nwbfiles)
str(cls.tempdir / f"unique_id_testing{j}.{suffix}.nwb") for j in range(num_nwbfiles)
]
for nwbfile_path, nwbfile in zip(cls.unique_id_nwbfile_paths, unique_id_nwbfiles):
with cls.BackendIOClass(path=nwbfile_path, mode="w") as io:
Expand Down Expand Up @@ -840,7 +841,7 @@ def setUpClass(cls):

suffix = IO_CLASSES_TO_BACKEND[cls.BackendIOClass]
cls.non_unique_id_nwbfile_paths = [
str(cls.tempdir / f"non_unique_id_testing{j}.nwb.{suffix}") for j in range(num_nwbfiles)
str(cls.tempdir / f"non_unique_id_testing{j}.{suffix}.nwb") for j in range(num_nwbfiles)
]
for nwbfile_path, nwbfile in zip(cls.non_unique_id_nwbfile_paths, non_unique_id_nwbfiles):
with cls.BackendIOClass(path=nwbfile_path, mode="w") as io:
Expand Down
6 changes: 3 additions & 3 deletions tests/unit_tests/test_ecephys.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,10 +133,10 @@ def test_pass(self):
assert check_electrical_series_reference_electrodes_table(electrical_series) is None

def test_trigger_check_electrical_series_reference_electrodes_table(self):
dyn_tab = DynamicTable("name", "desc")
dyn_tab.add_column("name", "desc")
dyn_tab = DynamicTable(name="name", description="desc")
dyn_tab.add_column("group_name", "desc")
for i in range(5):
dyn_tab.add_row(name=1)
dyn_tab.add_row(group_name=1)
dynamic_table_region = DynamicTableRegion(
name="electrodes", description="I am wrong", data=[0, 1, 2, 3, 4], table=dyn_tab
)
Expand Down
8 changes: 5 additions & 3 deletions tests/unit_tests/test_images.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ def test_pass_check_index_series_points_to_image():
name="images",
images=[gs_img],
description="An example collection.",
order_of_images=ImageReferences("order_of_images", [gs_img]),
order_of_images=ImageReferences(name="order_of_images", data=[gs_img]),
)

idx_series = IndexSeries(
Expand All @@ -92,7 +92,9 @@ def test_fail_check_index_series_points_to_image():
unit="n.a.",
)

idx_series = IndexSeries(
# Use __new__ and in_construct_mode=True to bypass the check in pynwb for deprecated indexed_timeseries
idx_series = IndexSeries.__new__(IndexSeries, in_construct_mode=True)
idx_series.__init__(
name="stimuli",
data=[0, 1, 0, 1],
indexed_timeseries=time_series,
Expand All @@ -105,7 +107,7 @@ def test_fail_check_index_series_points_to_image():
importance=Importance.BEST_PRACTICE_VIOLATION,
object_type="IndexSeries",
message="Pointing an IndexSeries to a TimeSeries will be deprecated. Please point to an Images container "
"instead.",
"instead.", # TODO - update message when PyNWB 3.0 is released
location="/",
check_function_name="check_index_series_points_to_image",
)
4 changes: 2 additions & 2 deletions tests/unit_tests/test_nwbfile_metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -548,7 +548,7 @@ def test_pass_check_subject_id_exist():


def test_check_processing_module_name():
processing_module = ProcessingModule("test", "desc")
processing_module = ProcessingModule(name="test", description="desc")
assert check_processing_module_name(processing_module) == InspectorMessage(
message=(
f"Processing module is named test. It is recommended to use the schema "
Expand All @@ -563,5 +563,5 @@ def test_check_processing_module_name():


def test_pass_check_processing_module_name():
processing_module = ProcessingModule("ecephys", "desc")
processing_module = ProcessingModule(name="ecephys", description="desc")
assert check_processing_module_name(processing_module) is None
20 changes: 11 additions & 9 deletions tests/unit_tests/test_time_series.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,16 +133,18 @@ def test_check_timestamps_match_first_dimension_special_skip(tmp_path):
data=np.empty(shape=(num_images, image_width, image_height, num_channels), dtype=dtype),
timestamps=[],
)
nwbfile.add_acquisition(image_series)
nwbfile.add_acquisition(
pynwb.image.IndexSeries(
name="IndexSeries",
unit="N/A",
data=[0, 1],
indexed_timeseries=image_series,
timestamps=[0.5, 0.6],
)

# Use __new__ and in_construct_mode=True to bypass the check in pynwb for deprecated indexed_timeseries
index_series = pynwb.image.IndexSeries.__new__(pynwb.image.IndexSeries, in_construct_mode=True)
index_series.__init__(
name="IndexSeries",
unit="N/A",
data=[0, 1],
indexed_timeseries=image_series,
timestamps=[0.5, 0.6],
)
nwbfile.add_acquisition(image_series)
nwbfile.add_acquisition(index_series)

with pynwb.NWBHDF5IO(path=nwbfile_path, mode="w") as io:
io.write(nwbfile)
Expand Down
Loading