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 support for NWB schema 2.6.0 and prepare PyNWB 2.3.0 #1611

Merged
merged 26 commits into from
Feb 22, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
2 changes: 1 addition & 1 deletion .github/workflows/run_coverage.yml
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ jobs:

- name: Run integration tests and generate coverage report
run: |
python -m coverage run -p test.py --integration
python -m coverage run -p test.py --integration --backwards
# validation CLI tests generate separate .coverage files that need to be merged
python -m coverage combine
python -m coverage xml # codecov uploader requires xml format
Expand Down
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
## Upcoming

### Enhancements and minor changes
- `Subject.age` can be input as a `timedelta`. @bendichter [#1590](https://github.com/NeurodataWithoutBorders/pynwb/pull/1590)
- `Subject.age` can be input as a `timedelta` type. @bendichter [#1590](https://github.com/NeurodataWithoutBorders/pynwb/pull/1590)
- Add `Subject.age__reference` field. @bendichter ([#1540](https://github.com/NeurodataWithoutBorders/pynwb/pull/1540))
- `IntracellularRecordingsTable.add_recording`: the `electrode` arg is now optional, and is automatically populated from the stimulus or response.
[#1597](https://github.com/NeurodataWithoutBorders/pynwb/pull/1597)
- Add module `pynwb.testing.mock.icephys` and corresponding tests. @bendichter
Expand Down
51 changes: 38 additions & 13 deletions src/pynwb/file.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ class Subject(NWBContainer):

__nwbfields__ = (
'age',
"age__reference",
'description',
'genotype',
'sex',
Expand All @@ -72,8 +73,20 @@ class Subject(NWBContainer):
'A timedelta will automatically be converted to The ISO 8601 Duration format.',
"default": None,
},
{'name': 'description', 'type': str,
'doc': 'A description of the subject, e.g., "mouse A10".', 'default': None},
{
"name": "age__reference",
"type": str,
"doc": "Age is with reference to this event. Can be 'birth' or 'gestational'. If reference is omitted, "
"then 'birth' is implied. Value can be None when read from an NWB file with schema version "
"2.0 to 2.5 where age__reference is missing.",
"default": "birth",
},
{
"name": "description",
"type": str,
"doc": 'A description of the subject, e.g., "mouse A10".',
"default": None,
},
{'name': 'genotype', 'type': str,
'doc': 'The genotype of the subject, e.g., "Sst-IRES-Cre/wt;Ai32(RCL-ChR2(H134R)_EYFP)/wt".',
'default': None},
Expand All @@ -94,18 +107,30 @@ class Subject(NWBContainer):
{'name': 'strain', 'type': str, 'doc': 'The strain of the subject, e.g., "C57BL/6J"', 'default': None},
)
def __init__(self, **kwargs):
keys_to_set = ("age",
"description",
"genotype",
"sex",
"species",
"subject_id",
"weight",
"date_of_birth",
"strain")
keys_to_set = (
"age",
"age__reference",
"description",
"genotype",
"sex",
"species",
"subject_id",
"weight",
"date_of_birth",
"strain",
)
args_to_set = popargs_to_dict(keys_to_set, kwargs)
kwargs['name'] = 'subject'
super().__init__(**kwargs)
super().__init__(name="subject", **kwargs)

# NOTE when the Subject I/O mapper (see pynwb.io.file.py) reads an age__reference value of None from an
# NWB 2.0-2.5 file, it sets the value to "unspecified" so that when Subject.__init__ is called, the incoming
# age__reference value is NOT replaced by the default value ("birth") specified in the docval.
# then we replace "unspecified" with None here. the user will never see the value "unspecified".
# the ONLY way that age__reference can now be None is if it is read as None from an NWB 2.0-2.5 file.
if self._in_construct_mode and args_to_set["age__reference"] == "unspecified":
args_to_set["age__reference"] = None
elif args_to_set["age__reference"] not in ("birth", "gestational"):
raise ValueError("age__reference, if supplied, must be 'birth' or 'gestational'.")

weight = args_to_set['weight']
if isinstance(weight, float):
Expand Down
14 changes: 14 additions & 0 deletions src/pynwb/io/file.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from .. import register_map
from ..file import NWBFile, Subject
from ..core import ScratchData
from .utils import get_nwb_version


@register_map(NWBFile)
Expand Down Expand Up @@ -220,3 +221,16 @@ def dateconversion(self, builder, manager):
datestr = dob_builder.data
date = dateutil_parse(datestr)
return date

@ObjectMapper.constructor_arg("age__reference")
def age_reference_none(self, builder, manager):
age_builder = builder.get("age")
age_reference = None
if age_builder is not None:
age_reference = age_builder["attributes"].get("reference")
if age_reference is None:
if get_nwb_version(builder) < (2, 6, 0):
return "unspecified" # this is handled specially in Subject.__init__
else:
return "birth"
return age_reference
27 changes: 27 additions & 0 deletions src/pynwb/io/utils.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import re
from typing import Tuple

from hdmf.build import Builder


def get_nwb_version(builder: Builder) -> Tuple[int, ...]:
"""Get the version of the NWB file from the root of the given builder, as a tuple.

If the "nwb_version" attribute on the root builder equals "2.5.1", then (2, 5, 1) is returned.
If the "nwb_version" attribute on the root builder equals "2.5.1-alpha", then (2, 5, 1) is returned.
oruebel marked this conversation as resolved.
Show resolved Hide resolved

:param builder: Any builder within an NWB file.
:type builder: Builder
:return: The version of the NWB file, as a tuple.
:rtype: tuple
:raises ValueError: if the 'nwb_version' attribute is missing from the root of the NWB file.
"""
temp_builder = builder
while temp_builder.parent is not None:
temp_builder = temp_builder.parent
root_builder = temp_builder
nwb_version = root_builder.attributes.get("nwb_version")
if nwb_version is None:
raise ValueError("'nwb_version' attribute is missing from the root of the NWB file.")
nwb_version = re.match(r"(\d+\.\d+\.\d+)", nwb_version)[0] # trim off any non-numeric symbols at end
return tuple([int(i) for i in nwb_version.split(".")])
21 changes: 21 additions & 0 deletions src/pynwb/testing/make_test_files.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import numpy as np
from pathlib import Path
from pynwb import NWBFile, NWBHDF5IO, __version__, TimeSeries, get_class, load_namespaces
from pynwb.file import Subject
from pynwb.image import ImageSeries
from pynwb.spec import NWBNamespaceBuilder, export_spec, NWBGroupSpec, NWBAttributeSpec

Expand Down Expand Up @@ -197,6 +198,23 @@ def _make_empty_with_extension():
_write(test_name, nwbfile)


def _make_subject_without_age_reference():
"""Create a test file without a value for age_reference."""
nwbfile = NWBFile(session_description='ADDME',
identifier='ADDME',
session_start_time=datetime.now().astimezone())
subject = Subject(
age="P90D",
description="A rat",
subject_id="RAT123",
)

nwbfile.subject = subject

test_name = 'subject_no_age__reference'
_write(test_name, nwbfile)


if __name__ == '__main__':
# install these versions of PyNWB and run this script to generate new files
# python src/pynwb/testing/make_test_files.py
Expand All @@ -221,3 +239,6 @@ def _make_empty_with_extension():
_make_imageseries_non_external_format()
_make_imageseries_nonmatch_starting_frame()
_make_empty_with_extension()

if __version__ == "2.2.0":
_make_subject_without_age_reference()
2 changes: 2 additions & 0 deletions test.py
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,8 @@ def run_integration_tests(verbose=True):
else:
logging.info('all classes have integration tests')

run_test_suite("tests/integration/utils", "integration utils tests", verbose=verbose)

# also test the validation script
run_test_suite("tests/validation", "validation tests", verbose=verbose)

Expand Down
Binary file not shown.
7 changes: 7 additions & 0 deletions tests/back_compat/test_read.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,3 +113,10 @@ def test_read_imageseries_nonmatch_starting_frame(self):
with NWBHDF5IO(str(f), 'r') as io:
read_nwbfile = io.read()
np.testing.assert_array_equal(read_nwbfile.acquisition['test_imageseries'].starting_frame, [1, 2, 3])

def test_read_subject_no_age__reference(self):
"""Test that reading a Subject without an age__reference set with NWB schema 2.5.0 sets the value to None"""
f = Path(__file__).parent / '2.2.0_subject_no_age__reference.nwb'
with NWBHDF5IO(str(f), 'r') as io:
read_nwbfile = io.read()
self.assertIsNone(read_nwbfile.subject.age__reference)
46 changes: 37 additions & 9 deletions tests/integration/hdf5/test_nwbfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -203,15 +203,43 @@ class TestSubjectIO(NWBH5IOMixin, TestCase):

def setUpContainer(self):
""" Return the test Subject """
return Subject(age='P90D',
description='An unfortunate rat',
genotype='WT',
sex='M',
species='Rattus norvegicus',
subject_id='RAT123',
weight='2 kg',
date_of_birth=datetime(1970, 1, 1, 12, tzinfo=tzutc()),
strain='my_strain')
return Subject(
age="P90D",
age__reference="gestational",
description="An unfortunate rat",
genotype="WT",
sex="M",
species="Rattus norvegicus",
subject_id="RAT123",
weight="2 kg",
date_of_birth=datetime(1970, 1, 1, 12, tzinfo=tzutc()),
strain="my_strain",
)

def addContainer(self, nwbfile):
""" Add the test Subject to the given NWBFile """
nwbfile.subject = self.container

def getContainer(self, nwbfile):
""" Return the test Subject from the given NWBFile """
return nwbfile.subject


class TestSubjectAgeReferenceNotSetIO(NWBH5IOMixin, TestCase):

def setUpContainer(self):
""" Return the test Subject """
return Subject(
age="P90D",
description="An unfortunate rat",
genotype="WT",
sex="M",
species="Rattus norvegicus",
subject_id="RAT123",
weight="2 kg",
date_of_birth=datetime(1970, 1, 1, 12, tzinfo=tzutc()),
strain="my_strain",
)

def addContainer(self, nwbfile):
""" Add the test Subject to the given NWBFile """
Expand Down
Empty file.
30 changes: 30 additions & 0 deletions tests/integration/utils/test_io_utils.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
"""Tests related to pynwb.io.utils."""
import pytest

from hdmf.build import GroupBuilder
from pynwb.io.utils import get_nwb_version
from pynwb.testing import TestCase


class TestGetNWBVersion(TestCase):

def test_get_nwb_version(self):
"""Get the NWB version from a builder."""
builder1 = GroupBuilder(name="root")
builder1.set_attribute(name="nwb_version", value="2.0.0")
builder2 = GroupBuilder(name="another")
builder1.set_group(builder2)
assert get_nwb_version(builder1) == (2, 0, 0)
assert get_nwb_version(builder2) == (2, 0, 0)

def test_get_nwb_version_missing(self):
"""Get the NWB version from a builder where the root builder does not have an nwb_version attribute."""
builder1 = GroupBuilder(name="root")
builder2 = GroupBuilder(name="another")
builder1.set_group(builder2)

with pytest.raises(ValueError, match="'nwb_version' attribute is missing from the root of the NWB file."):
get_nwb_version(builder1)

with pytest.raises(ValueError, match="'nwb_version' attribute is missing from the root of the NWB file."):
get_nwb_version(builder1)
67 changes: 49 additions & 18 deletions tests/unit/test_file.py
Original file line number Diff line number Diff line change
Expand Up @@ -437,29 +437,35 @@ def test_multi_publications(self):

class SubjectTest(TestCase):
def setUp(self):
self.subject = Subject(age='P90D',
description='An unfortunate rat',
genotype='WT',
sex='M',
species='Rattus norvegicus',
subject_id='RAT123',
weight='2 kg',
date_of_birth=datetime(2017, 5, 1, 12, tzinfo=tzlocal()),
strain='my_strain')
self.subject = Subject(
age='P90D',
age__reference="birth",
description='An unfortunate rat',
genotype='WT',
sex='M',
species='Rattus norvegicus',
subject_id='RAT123',
weight='2 kg',
date_of_birth=datetime(2017, 5, 1, 12, tzinfo=tzlocal()),
strain='my_strain',
)
self.start = datetime(2017, 5, 1, 12, tzinfo=tzlocal())
self.path = 'nwbfile_test.h5'
self.nwbfile = NWBFile('a test session description for a test NWBFile',
'FILE123',
self.start,
experimenter='A test experimenter',
lab='a test lab',
institution='a test institution',
experiment_description='a test experiment description',
session_id='test1',
subject=self.subject)
self.nwbfile = NWBFile(
'a test session description for a test NWBFile',
'FILE123',
self.start,
experimenter='A test experimenter',
lab='a test lab',
institution='a test institution',
experiment_description='a test experiment description',
session_id='test1',
subject=self.subject,
)

def test_constructor(self):
self.assertEqual(self.subject.age, 'P90D')
self.assertEqual(self.subject.age__reference, "birth")
self.assertEqual(self.subject.description, 'An unfortunate rat')
self.assertEqual(self.subject.genotype, 'WT')
self.assertEqual(self.subject.sex, 'M')
Expand All @@ -479,6 +485,31 @@ def test_weight_float(self):
)
self.assertEqual(subject.weight, '2.3 kg')

def test_age_reference_arg_check(self):
with self.assertRaisesWith(ValueError, "age__reference, if supplied, must be 'birth' or 'gestational'."):
Subject(subject_id='RAT123', age='P90D', age__reference='brth')

def test_age_regression_1(self):
subject = Subject(
age='P90D',
description='An unfortunate rat',
subject_id='RAT123',
)

self.assertEqual(subject.age, 'P90D')
self.assertEqual(subject.age__reference, "birth")
self.assertEqual(subject.description, 'An unfortunate rat')
self.assertEqual(subject.subject_id, 'RAT123')

def test_age_regression_2(self):
subject = Subject(
description='An unfortunate rat',
subject_id='RAT123',
)

self.assertEqual(subject.description, 'An unfortunate rat')
self.assertEqual(subject.subject_id, 'RAT123')

def test_subject_age_duration(self):
subject = Subject(
subject_id='RAT123',
Expand Down