diff --git a/CHANGELOG.md b/CHANGELOG.md index 52a8c256e..df751a5bc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,18 +4,18 @@ ### Documentation and tutorial enhancements: - Support explicit ordering of sphinx gallery tutorials in the docs. @oruebel (#1504), @bdichter (#1495) -- Add developer guide on how to create a new tutorial. @oruebel (#1504) +- Add developer guide on how to create a new tutorial. @oruebel (#1504) - Add images tutorial. @weiglszonja (#1470) - Add example code for s3fs in the streaming tutorial. @bdichter (#1499) ### Enhancements and minor changes - Update coverage workflow, report separate unit vs integration coverage. @rly (#1509) -- Enable passing an S3File created through s3fs, which provides a method for reading an NWB file directly +- Enable passing an S3File created through s3fs, which provides a method for reading an NWB file directly from s3 that is an alternative to ros3. This required relaxing of `NWBHDF5IO` input validation. The `path` - arg is not needed if `file` is provided. `mode` now has a default value of "r". + arg is not needed if `file` is provided. `mode` now has a default value of "r". @bendichter (#1499) - Added a method to `NWBMixin` that only raises an error when a check is violated on instance creation, - otherwise throws a warning when reading from a file. The new checks in `ImageSeries` when `external_file` + otherwise throws a warning when reading from a file. The new checks in `ImageSeries` when `external_file` is provided is used with this method to ensure that that files with invalid data can be read, but prohibits the user from creating new instances when these checks are violated. @weiglszonja (#1516) diff --git a/src/pynwb/file.py b/src/pynwb/file.py index bde73349e..90db64a5c 100644 --- a/src/pynwb/file.py +++ b/src/pynwb/file.py @@ -2,7 +2,6 @@ from dateutil.tz import tzlocal from collections.abc import Iterable from warnings import warn -import copy as _copy import numpy as np import pandas as pd @@ -102,6 +101,112 @@ def __init__(self, **kwargs): setattr(self, key, val) +_add_electrode_docval = ( + {'name': 'x', 'type': 'float', 'doc': 'the x coordinate of the position (+x is posterior)', + 'default': None}, + {'name': 'y', 'type': 'float', 'doc': 'the y coordinate of the position (+y is inferior)', 'default': None}, + {'name': 'z', 'type': 'float', 'doc': 'the z coordinate of the position (+z is right)', 'default': None}, + {'name': 'imp', 'type': 'float', 'doc': 'the impedance of the electrode, in ohms', 'default': None}, + {'name': 'location', 'type': str, + 'doc': 'the location of electrode within the subject e.g. brain region. Required.', + 'default': None}, + {'name': 'filtering', 'type': str, + 'doc': 'description of hardware filtering, including the filter name and frequency cutoffs', + 'default': None}, + {'name': 'group', 'type': ElectrodeGroup, + 'doc': 'the ElectrodeGroup object to add to this NWBFile. Required.', + 'default': None}, + {'name': 'id', 'type': int, 'doc': 'a unique identifier for the electrode', 'default': None}, + {'name': 'rel_x', 'type': 'float', 'doc': 'the x coordinate within the electrode group', 'default': None}, + {'name': 'rel_y', 'type': 'float', 'doc': 'the y coordinate within the electrode group', 'default': None}, + {'name': 'rel_z', 'type': 'float', 'doc': 'the z coordinate within the electrode group', 'default': None}, + {'name': 'reference', 'type': str, 'doc': 'Description of the reference electrode and/or reference scheme\ + used for this electrode, e.g.,"stainless steel skull screw" or "online common average referencing". ', + 'default': None}, + {'name': 'enforce_unique_id', 'type': bool, 'doc': 'enforce that the id in the table must be unique', + 'default': True} +) + + +class ElectrodeTable(DynamicTable): + + # NOTE: ElectrodeTable is not yet a standalone type in the NWB schema (with its own neurodata_type_def) + # ElectrodeTable parameters are fixed + + __columns__ = ( + {'name': 'location', 'description': 'the location of channel within the subject e.g. brain region', + 'required': True}, + {'name': 'group', 'description': 'a reference to the ElectrodeGroup this electrode is a part of', + 'required': True}, + {'name': 'group_name', 'description': 'the name of the ElectrodeGroup this electrode is a part of', + 'required': True}, + {'name': 'x', 'description': 'the x coordinate of the position (+x is posterior)'}, + {'name': 'y', 'description': 'the y coordinate of the position (+y is inferior)'}, + {'name': 'z', 'description': 'the z coordinate of the position (+z is right)'}, + {'name': 'imp', 'description': 'the impedance of the electrode, in ohms'}, + {'name': 'filtering', 'description': 'description of hardware filtering, including the filter name ' + 'and frequency cutoffs'}, + {'name': 'rel_x', 'description': 'the x coordinate within the electrode group'}, + {'name': 'rel_y', 'description': 'the y coordinate within the electrode group'}, + {'name': 'rel_z', 'description': 'the z coordinate within the electrode group'}, + {'name': 'reference', 'description': 'Description of the reference electrode and/or reference scheme ' + 'used for this electrode, e.g.,"stainless steel skull screw" or "online common average referencing".'}, + ) + + @docval(*get_docval(DynamicTable.__init__, "id", "columns", "colnames"), + allow_positional=AllowPositional.WARNING) + def __init__(self, **kwargs): + super().__init__( + name='electrodes', + description='metadata about extracellular electrodes', + **kwargs + ) + + # NOTE _add_electrode_docval is defined outside the class so it can be used by NWBFile.add_electrode + @docval(*_add_electrode_docval, + allow_extra=True, + allow_positional=AllowPositional.WARNING) + def add_electrode(self, **kwargs): + self.add_row(**kwargs) + + # NOTE _add_electrode_docval is defined outside the class so it can be used by NWBFile.add_electrode + @docval(*_add_electrode_docval, + allow_extra=True, + allow_positional=AllowPositional.WARNING) + def add_row(self, **kwargs): + """ + Add an electrode. Optional columns are + See :py:meth:`~hdmf.common.DynamicTable.add_row` for more details. + + Required fields are *location* and + *group* and any columns that have been added + (through calls to `add_electrode_column`). + """ + + # NOTE location and group are required arguments. in PyNWB 2.1.0, 'x', 'y', and 'z' became optional arguments, + # and in order to avoid breaking API changes, the order of the arguments needed to be maintained even though + # these optional arguments came before the required arguments, so in docval these required arguments are + # displayed as optional when really they are required. this should be changed when positional arguments + # are no longer allowed + if not kwargs['location']: + raise ValueError("The 'location' argument is required when creating an electrode.") + if not kwargs['group']: + raise ValueError("The 'group' argument is required when creating an electrode.") + + if kwargs.get('group_name', None) is None: + kwargs['group_name'] = kwargs['group'].name + + super().add_row(**kwargs) + + def copy(self): + """ + Return a copy of this ElectrodeTable. + This is useful for linking. + """ + kwargs = dict(id=self.id, columns=self.columns, colnames=self.colnames) + return self.__class__(**kwargs) + + @register_class('NWBFile', CORE_NAMESPACE) class NWBFile(MultiContainerInterface): """ @@ -607,29 +712,7 @@ def add_electrode_column(self, **kwargs): self.__check_electrodes() self.electrodes.add_column(**kwargs) - @docval({'name': 'x', 'type': float, 'doc': 'the x coordinate of the position (+x is posterior)', - 'default': None}, - {'name': 'y', 'type': float, 'doc': 'the y coordinate of the position (+y is inferior)', 'default': None}, - {'name': 'z', 'type': float, 'doc': 'the z coordinate of the position (+z is right)', 'default': None}, - {'name': 'imp', 'type': float, 'doc': 'the impedance of the electrode, in ohms', 'default': None}, - {'name': 'location', 'type': str, - 'doc': 'the location of electrode within the subject e.g. brain region. Required.', - 'default': None}, - {'name': 'filtering', 'type': str, - 'doc': 'description of hardware filtering, including the filter name and frequency cutoffs', - 'default': None}, - {'name': 'group', 'type': ElectrodeGroup, - 'doc': 'the ElectrodeGroup object to add to this NWBFile. Required.', - 'default': None}, - {'name': 'id', 'type': int, 'doc': 'a unique identifier for the electrode', 'default': None}, - {'name': 'rel_x', 'type': float, 'doc': 'the x coordinate within the electrode group', 'default': None}, - {'name': 'rel_y', 'type': float, 'doc': 'the y coordinate within the electrode group', 'default': None}, - {'name': 'rel_z', 'type': float, 'doc': 'the z coordinate within the electrode group', 'default': None}, - {'name': 'reference', 'type': str, 'doc': 'Description of the reference electrode and/or reference scheme\ - used for this electrode, e.g.,"stainless steel skull screw" or "online common average referencing". ', - 'default': None}, - {'name': 'enforce_unique_id', 'type': bool, 'doc': 'enforce that the id in the table must be unique', - 'default': True}, + @docval(*_add_electrode_docval, allow_extra=True, allow_positional=AllowPositional.WARNING) def add_electrode(self, **kwargs): @@ -642,42 +725,7 @@ def add_electrode(self, **kwargs): (through calls to `add_electrode_columns`). """ self.__check_electrodes() - d = _copy.copy(kwargs['data']) if kwargs.get('data') is not None else kwargs - - # NOTE location and group are required arguments. in PyNWB 2.1.0 we made x, y, z optional arguments, and - # in order to avoid breaking API changes, the order of the arguments needed to be maintained even though - # these optional arguments came before the required arguments, so in docval these required arguments are - # displayed as optional when really they are required. this should be changed when positional arguments - # are not allowed - if not d['location']: - raise ValueError("The 'location' argument is required when creating an electrode.") - if not kwargs['group']: - raise ValueError("The 'group' argument is required when creating an electrode.") - if d.get('group_name', None) is None: - d['group_name'] = d['group'].name - - new_cols = [('x', 'the x coordinate of the position (+x is posterior)'), - ('y', 'the y coordinate of the position (+y is inferior)'), - ('z', 'the z coordinate of the position (+z is right)'), - ('imp', 'the impedance of the electrode, in ohms'), - ('filtering', 'description of hardware filtering, including the filter name and frequency cutoffs'), - ('rel_x', 'the x coordinate within the electrode group'), - ('rel_y', 'the y coordinate within the electrode group'), - ('rel_z', 'the z coordinate within the electrode group'), - ('reference', 'Description of the reference electrode and/or reference scheme used for this \ - electrode, e.g.,"stainless steel skull screw" or "online common average referencing".') - ] - - # add column if the arg is supplied and column does not yet exist - # do not pass arg to add_row if arg is not supplied - for col_name, col_doc in new_cols: - if kwargs[col_name] is not None: - if col_name not in self.electrodes: - self.electrodes.add_column(col_name, col_doc) - else: - d.pop(col_name) # remove args from d if not set - - self.electrodes.add_row(**d) + self.electrodes.add_electrode(**kwargs) @docval({'name': 'region', 'type': (slice, list, tuple), 'doc': 'the indices of the table'}, {'name': 'description', 'type': str, 'doc': 'a brief description of what this electrode is'}, @@ -772,7 +820,7 @@ def add_invalid_time_interval(self, **kwargs): self.__check_invalid_times() self.invalid_times.add_interval(**kwargs) - @docval({'name': 'electrode_table', 'type': DynamicTable, 'doc': 'the ElectrodeTable for this file'}) + @docval({'name': 'electrode_table', 'type': ElectrodeTable, 'doc': 'the ElectrodeTable for this file'}) def set_electrode_table(self, **kwargs): """ Set the electrode table of this NWBFile to an existing ElectrodeTable @@ -783,6 +831,11 @@ def set_electrode_table(self, **kwargs): electrode_table = getargs('electrode_table', kwargs) self.electrodes = electrode_table + @electrodes.setter # can I overwrite the generated one? probably not. + def electrodes(self, v): + # TODO cast the DynamicTable as an ElectrodeTable + self.electrodes = v + def _check_sweep_table(self): """ Create a SweepTable if not yet done. @@ -1116,16 +1169,6 @@ def _tablefunc(table_name, description, columns): return t -def ElectrodeTable(name='electrodes', - description='metadata about extracellular electrodes'): - return _tablefunc(name, description, - [('location', 'the location of channel within the subject e.g. brain region'), - ('group', 'a reference to the ElectrodeGroup this electrode is a part of'), - ('group_name', 'the name of the ElectrodeGroup this electrode is a part of') - ] - ) - - def TrialTable(name='trials', description='metadata about experimental trials'): return _tablefunc(name, description, ['start_time', 'stop_time']) diff --git a/tests/unit/test_file.py b/tests/unit/test_file.py index 6775180eb..6dd950280 100644 --- a/tests/unit/test_file.py +++ b/tests/unit/test_file.py @@ -264,67 +264,6 @@ def test_add_electrode(self): self.assertEqual(elec.iloc[0]['filtering'], 'none') self.assertEqual(elec.iloc[0]['group'], group) - def test_add_electrode_some_opt(self): - dev1 = self.nwbfile.create_device(name='dev1') - group = self.nwbfile.create_electrode_group( - name='tetrode1', - description='tetrode description', - location='tetrode location', - device=dev1 - ) - self.nwbfile.add_electrode( - x=1.0, y=2.0, z=3.0, - imp=-1.0, - location='CA1', - filtering='none', - group=group, - id=1, - rel_x=4.0, rel_y=5.0, rel_z=6.0, - reference='ref1' - ) - self.nwbfile.add_electrode( - x=1.0, y=2.0, z=3.0, - imp=-1.0, - location='CA1', - filtering='none', - group=group, - id=2, - rel_x=7.0, rel_y=8.0, rel_z=9.0, - reference='ref2' - ) - elec = self.nwbfile.electrodes[0] - self.assertEqual(elec.iloc[0]['rel_x'], 4.0) - self.assertEqual(elec.iloc[0]['rel_y'], 5.0) - self.assertEqual(elec.iloc[0]['rel_z'], 6.0) - self.assertEqual(elec.iloc[0]['reference'], 'ref1') - elec = self.nwbfile.electrodes[1] - self.assertEqual(elec.iloc[0]['rel_x'], 7.0) - self.assertEqual(elec.iloc[0]['rel_y'], 8.0) - self.assertEqual(elec.iloc[0]['rel_z'], 9.0) - self.assertEqual(elec.iloc[0]['reference'], 'ref2') - - def test_add_electrode_missing_location(self): - """ - Test the case where the user creates an electrode table region with - indexes that are out of range of the amount of electrodes added. - """ - nwbfile = NWBFile('a', 'b', datetime.now(tzlocal())) - device = nwbfile.create_device('a') - elecgrp = nwbfile.create_electrode_group('a', 'b', device=device, location='a') - msg = "The 'location' argument is required when creating an electrode." - with self.assertRaisesWith(ValueError, msg): - nwbfile.add_electrode(group=elecgrp, id=0) - - def test_add_electrode_missing_group(self): - """ - Test the case where the user creates an electrode table region with - indexes that are out of range of the amount of electrodes added. - """ - nwbfile = NWBFile('a', 'b', datetime.now(tzlocal())) - msg = "The 'group' argument is required when creating an electrode." - with self.assertRaisesWith(ValueError, msg): - nwbfile.add_electrode(location='a', id=0) - def test_all_children(self): ts1 = TimeSeries('test_ts1', [0, 1, 2, 3, 4, 5], 'grams', timestamps=[0.0, 0.1, 0.2, 0.3, 0.4, 0.5]) ts2 = TimeSeries('test_ts2', [0, 1, 2, 3, 4, 5], 'grams', timestamps=[0.0, 0.1, 0.2, 0.3, 0.4, 0.5]) @@ -435,6 +374,156 @@ def test_multi_publications(self): self.assertTupleEqual(self.nwbfile.related_publications, ('pub1', 'pub2')) +class TestElectrodeTable(TestCase): + + def setUp(self): + self.nwbfile = NWBFile( + session_description='a test session description for a test NWBFile', + identifier='FILE123', + session_start_time=datetime(2017, 5, 1, 12, 0, 0, tzinfo=tzlocal()) + ) + + def test_set_electrodes(self): + electrodes = ElectrodeTable() + self.nwbfile.electrodes = electrodes + + def test_electrodes_add_row(self): + dev1 = self.nwbfile.create_device(name='dev1') + group = self.nwbfile.create_electrode_group( + name='tetrode1', + description='tetrode description', + location='tetrode location', + device=dev1 + ) + + table = ElectrodeTable() + table.add_row( + x=1.0, y=2.0, z=3.0, + imp=-1.0, + location='CA1', + filtering='none', + group=group, + id=1 + ) + + elec = table[0] + self.assertEqual(elec.index[0], 1) + self.assertEqual(elec.iloc[0]['x'], 1.0) + self.assertEqual(elec.iloc[0]['y'], 2.0) + self.assertEqual(elec.iloc[0]['z'], 3.0) + self.assertEqual(elec.iloc[0]['location'], 'CA1') + self.assertEqual(elec.iloc[0]['filtering'], 'none') + self.assertEqual(elec.iloc[0]['group'], group) + + def test_electrodes_add_electrode(self): + dev1 = self.nwbfile.create_device(name='dev1') + group = self.nwbfile.create_electrode_group( + name='tetrode1', + description='tetrode description', + location='tetrode location', + device=dev1 + ) + + table = ElectrodeTable() + table.add_electrode( + x=1.0, y=2.0, z=3.0, + imp=-1.0, + location='CA1', + filtering='none', + group=group, + id=1 + ) + + elec = table[0] + self.assertEqual(elec.index[0], 1) + self.assertEqual(elec.iloc[0]['x'], 1.0) + self.assertEqual(elec.iloc[0]['y'], 2.0) + self.assertEqual(elec.iloc[0]['z'], 3.0) + self.assertEqual(elec.iloc[0]['location'], 'CA1') + self.assertEqual(elec.iloc[0]['filtering'], 'none') + self.assertEqual(elec.iloc[0]['group'], group) + + def test_add_electrode_opt_x(self): + dev1 = self.nwbfile.create_device(name='dev1') + group = self.nwbfile.create_electrode_group( + name='tetrode1', + description='tetrode description', + location='tetrode location', + device=dev1 + ) + table = ElectrodeTable() + table.add_electrode( + x=1.0, + location='CA1', + group=group, + ) + # confirm that x is added and y is not + elec = table[0] + self.assertEqual(elec.index[0], 0) + self.assertEqual(elec.iloc[0]['x'], 1.0) + self.assertTrue('y' not in elec.iloc[0]) + + def test_add_electrode_opt(self): + dev1 = self.nwbfile.create_device(name='dev1') + group = self.nwbfile.create_electrode_group( + name='tetrode1', + description='tetrode description', + location='tetrode location', + device=dev1 + ) + table = ElectrodeTable() + table.add_electrode( + x=1.0, y=2.0, z=3.0, + imp=-1.0, + location='CA1', + filtering='none', + group=group, + id=1, + rel_x=4.0, rel_y=5.0, rel_z=6.0, + reference='ref1' + ) + table.add_electrode( + x=1.0, y=2.0, z=3.0, + imp=-1.0, + location='CA1', + filtering='none', + group=group, + id=2, + rel_x=7.0, rel_y=8.0, rel_z=9.0, + reference='ref2' + ) + elec = table[0] + self.assertEqual(elec.iloc[0]['rel_x'], 4.0) + self.assertEqual(elec.iloc[0]['rel_y'], 5.0) + self.assertEqual(elec.iloc[0]['rel_z'], 6.0) + self.assertEqual(elec.iloc[0]['reference'], 'ref1') + elec = table[1] + self.assertEqual(elec.iloc[0]['rel_x'], 7.0) + self.assertEqual(elec.iloc[0]['rel_y'], 8.0) + self.assertEqual(elec.iloc[0]['rel_z'], 9.0) + self.assertEqual(elec.iloc[0]['reference'], 'ref2') + + def test_add_electrode_missing_location(self): + """ + Test the case where the user tries to add an electrode without a location. + """ + device = self.nwbfile.create_device(name='a') + elecgrp = self.nwbfile.create_electrode_group(name='a', description='b', device=device, location='a') + table = ElectrodeTable() + msg = "The 'location' argument is required when creating an electrode." + with self.assertRaisesWith(ValueError, msg): + table.add_electrode(group=elecgrp) + + def test_add_electrode_missing_group(self): + """ + Test the case where the user tries to add an electrode without a group. + """ + table = ElectrodeTable() + msg = "The 'group' argument is required when creating an electrode." + with self.assertRaisesWith(ValueError, msg): + table.add_electrode(location='a') + + class SubjectTest(TestCase): def setUp(self): self.subject = Subject(age='P90D',