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.

Sign up for GitHub

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

[WIP] Create ElectrodeTable class, move add_electrode code #1539

Open
wants to merge 5 commits into
base: dev
Choose a base branch
from
Open

[WIP] Create ElectrodeTable class, move add_electrode code #1539

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
eae0ba8
Create ElectrodeTable class, move add_electrode code
rly Jul 8, 2022
63b3b6f
Update changelog
rly Jul 8, 2022
d383ca5
Fix flake8
rly Jul 8, 2022
6ab3527
temp - Move ElectrodeTable class
rly Jul 8, 2022
d4f7004
Merge branch 'dev' into fix/electrodetable_x
rly Aug 12, 2022
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
8 changes: 4 additions & 4 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
185 changes: 114 additions & 71 deletions src/pynwb/file.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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):
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm wondering whether it would make sense to define the ElectrodeTable in ecphys.py instead and import it in file.py


# 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):
"""
Expand Down Expand Up @@ -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):
Expand All @@ -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'},
Expand Down Expand Up @@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -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'])

Expand Down
Loading