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 warnings when using positional arguments in container constructors #1972

Merged
merged 4 commits into from
Oct 24, 2024
Merged
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
### Breaking changes

### Enhancements and minor changes
- Added warnings when using positional arguments in `Container` constructor methods. Positional arguments will raise errors in the next major release. @stephprince [#1972](https://github.com/NeurodataWithoutBorders/pynwb/pull/1972)

## PyNWB 2.8.3 (Upcoming)

Expand Down
20 changes: 13 additions & 7 deletions src/pynwb/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

from hdmf.utils import docval, popargs_to_dict, get_docval, popargs
from hdmf.common import DynamicTable, VectorData
from hdmf.utils import get_data_shape
from hdmf.utils import get_data_shape, AllowPositional

from . import register_class, CORE_NAMESPACE
from .core import NWBDataInterface, MultiContainerInterface, NWBData
Expand All @@ -33,7 +33,8 @@ class ProcessingModule(MultiContainerInterface):
@docval({'name': 'name', 'type': str, 'doc': 'The name of this processing module'},
{'name': 'description', 'type': str, 'doc': 'Description of this processing module'},
{'name': 'data_interfaces', 'type': (list, tuple, dict),
'doc': 'NWBDataInterfaces that belong to this ProcessingModule', 'default': None})
'doc': 'NWBDataInterfaces that belong to this ProcessingModule', 'default': None},
allow_positional=AllowPositional.WARNING,)
def __init__(self, **kwargs):
description, data_interfaces = popargs("description", "data_interfaces", kwargs)
super().__init__(**kwargs)
Expand Down Expand Up @@ -143,7 +144,8 @@ class TimeSeries(NWBDataInterface):
'distinct moments in time. Times of image presentations would be "step" because the picture '
'remains the same until the next time-point. This field is optional, but is useful in providing '
'information about the underlying data. It may inform the way this data is interpreted, the way it '
'is visualized, and what analysis methods are applicable.'})
'is visualized, and what analysis methods are applicable.'},
allow_positional=AllowPositional.WARNING,)
def __init__(self, **kwargs):
"""Create a TimeSeries object
"""
Expand Down Expand Up @@ -375,7 +377,8 @@ class Image(NWBData):
{'name': 'data', 'type': ('array_data', 'data'), 'doc': 'data of image. Dimensions: x, y [, r,g,b[,a]]',
'shape': ((None, None), (None, None, 3), (None, None, 4))},
{'name': 'resolution', 'type': float, 'doc': 'pixels / cm', 'default': None},
{'name': 'description', 'type': str, 'doc': 'description of image', 'default': None})
{'name': 'description', 'type': str, 'doc': 'description of image', 'default': None},
allow_positional=AllowPositional.WARNING,)
def __init__(self, **kwargs):
args_to_set = popargs_to_dict(("resolution", "description"), kwargs)
super().__init__(**kwargs)
Expand All @@ -392,7 +395,8 @@ class ImageReferences(NWBData):
__nwbfields__ = ('data', )

@docval({'name': 'name', 'type': str, 'doc': 'The name of this ImageReferences object.'},
{'name': 'data', 'type': 'array_data', 'doc': 'The images in order.'},)
{'name': 'data', 'type': 'array_data', 'doc': 'The images in order.'},
allow_positional=AllowPositional.WARNING,)
def __init__(self, **kwargs):
# NOTE we do not use the docval shape validator here because it will recognize a list of P MxN images as
# having shape (P, M, N)
Expand Down Expand Up @@ -424,7 +428,8 @@ class Images(MultiContainerInterface):
{'name': 'images', 'type': 'array_data', 'doc': 'image objects', 'default': None},
{'name': 'description', 'type': str, 'doc': 'description of images', 'default': 'no description'},
{'name': 'order_of_images', 'type': ImageReferences,
'doc': 'Ordered dataset of references to Image objects stored in the parent group.', 'default': None},)
'doc': 'Ordered dataset of references to Image objects stored in the parent group.', 'default': None},
allow_positional=AllowPositional.WARNING,)
def __init__(self, **kwargs):

args_to_set = popargs_to_dict(("description", "images", "order_of_images"), kwargs)
Expand Down Expand Up @@ -617,7 +622,8 @@ class TimeSeriesReferenceVectorData(VectorData):
'default': "Column storing references to a TimeSeries (rows). For each TimeSeries this "
"VectorData column stores the start_index and count to indicate the range in time "
"to be selected as well as an object reference to the TimeSeries."},
*get_docval(VectorData.__init__, 'data'))
*get_docval(VectorData.__init__, 'data'),
allow_positional=AllowPositional.WARNING,)
def __init__(self, **kwargs):
super().__init__(**kwargs)
# CAUTION: Define any logic specific for init in the self._init_internal function, not here!
Expand Down
9 changes: 5 additions & 4 deletions src/pynwb/behavior.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import warnings

from hdmf.utils import docval, popargs, get_docval, get_data_shape
from hdmf.utils import docval, popargs, get_docval, get_data_shape, AllowPositional

from . import register_class, CORE_NAMESPACE
from .core import MultiContainerInterface
Expand Down Expand Up @@ -33,13 +33,14 @@ class SpatialSeries(TimeSeries):
{'name': 'unit', 'type': str, 'doc': 'The base unit of measurement (should be SI unit)',
'default': 'meters'},
*get_docval(TimeSeries.__init__, 'conversion', 'resolution', 'timestamps', 'starting_time', 'rate',
'comments', 'description', 'control', 'control_description', 'offset'))
'comments', 'description', 'control', 'control_description', 'offset'),
allow_positional=AllowPositional.WARNING,)
def __init__(self, **kwargs):
"""
Create a SpatialSeries TimeSeries dataset
"""
name, data, bounds, reference_frame, unit = popargs('name', 'data', 'bounds', 'reference_frame', 'unit', kwargs)
super().__init__(name, data, unit, **kwargs)
name, data, bounds, reference_frame = popargs('name', 'data', 'bounds', 'reference_frame', kwargs)
super().__init__(name=name, data=data, **kwargs)

# NWB 2.5 restricts length of second dimension to be <= 3
allowed_data_shapes = ((None, ), (None, 1), (None, 2), (None, 3))
Expand Down
8 changes: 5 additions & 3 deletions src/pynwb/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from hdmf.container import AbstractContainer, MultiContainerInterface as hdmf_MultiContainerInterface, Table
from hdmf.common import DynamicTable, DynamicTableRegion # noqa: F401
from hdmf.common import VectorData, VectorIndex, ElementIdentifiers # noqa: F401
from hdmf.utils import docval, popargs
from hdmf.utils import docval, popargs, AllowPositional
from hdmf.utils import LabelledDict # noqa: F401

from . import CORE_NAMESPACE, register_class
Expand Down Expand Up @@ -78,7 +78,8 @@ class NWBDataInterface(NWBContainer):
class NWBData(NWBMixin, Data):

@docval({'name': 'name', 'type': str, 'doc': 'the name of this container'},
{'name': 'data', 'type': ('scalar_data', 'array_data', 'data', Data), 'doc': 'the source of the data'})
{'name': 'data', 'type': ('scalar_data', 'array_data', 'data', Data), 'doc': 'the source of the data'},
allow_positional=AllowPositional.WARNING,)
def __init__(self, **kwargs):
super().__init__(**kwargs)
self.__data = kwargs['data']
Expand Down Expand Up @@ -123,7 +124,8 @@ class ScratchData(NWBData):
{'name': 'data', 'type': ('scalar_data', 'array_data', 'data', Data), 'doc': 'the source of the data'},
{'name': 'notes', 'type': str,
'doc': 'notes about the data. This argument will be deprecated. Use description instead', 'default': ''},
{'name': 'description', 'type': str, 'doc': 'notes about the data', 'default': None})
{'name': 'description', 'type': str, 'doc': 'notes about the data', 'default': None},
allow_positional=AllowPositional.WARNING,)
def __init__(self, **kwargs):
notes, description = popargs('notes', 'description', kwargs)
super().__init__(**kwargs)
Expand Down
5 changes: 3 additions & 2 deletions src/pynwb/device.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from hdmf.utils import docval, popargs
from hdmf.utils import docval, popargs, AllowPositional

from . import register_class, CORE_NAMESPACE
from .core import NWBContainer
Expand All @@ -19,7 +19,8 @@ class Device(NWBContainer):
'doc': 'Description of the device (e.g., model, firmware version, processing software version, etc.)',
'default': None},
{'name': 'manufacturer', 'type': str, 'doc': 'the name of the manufacturer of this device',
'default': None})
'default': None},
allow_positional=AllowPositional.WARNING,)
def __init__(self, **kwargs):
description, manufacturer = popargs('description', 'manufacturer', kwargs)
super().__init__(**kwargs)
Expand Down
17 changes: 11 additions & 6 deletions src/pynwb/ecephys.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

from hdmf.common import DynamicTableRegion
from hdmf.data_utils import DataChunkIterator, assertEqualShape
from hdmf.utils import docval, popargs, get_docval, popargs_to_dict, get_data_shape
from hdmf.utils import docval, popargs, get_docval, popargs_to_dict, get_data_shape, AllowPositional

from . import register_class, CORE_NAMESPACE
from .base import TimeSeries
Expand All @@ -29,7 +29,8 @@ class ElectrodeGroup(NWBContainer):
{'name': 'position', 'type': 'array_data',
'doc': 'Compound dataset with stereotaxic position of this electrode group (x, y, z). '
'The data array must have three elements or the dtype of the '
'array must be ``(float, float, float)``', 'default': None})
'array must be ``(float, float, float)``', 'default': None},
allow_positional=AllowPositional.WARNING,)
def __init__(self, **kwargs):
args_to_set = popargs_to_dict(('description', 'location', 'device', 'position'), kwargs)
super().__init__(**kwargs)
Expand Down Expand Up @@ -91,7 +92,8 @@ class ElectricalSeries(TimeSeries):
"filter is unknown, then this value could be 'Low-pass filter at 300 Hz'. If a non-standard filter "
"type is used, provide as much detail about the filter properties as possible.", 'default': None},
*get_docval(TimeSeries.__init__, 'resolution', 'conversion', 'timestamps', 'starting_time', 'rate',
'comments', 'description', 'control', 'control_description', 'offset'))
'comments', 'description', 'control', 'control_description', 'offset'),
allow_positional=AllowPositional.WARNING,)
def __init__(self, **kwargs):
args_to_set = popargs_to_dict(('electrodes', 'channel_conversion', 'filtering'), kwargs)

Expand Down Expand Up @@ -134,7 +136,8 @@ class SpikeEventSeries(ElectricalSeries):
'doc': 'Timestamps for samples stored in data'},
*get_docval(ElectricalSeries.__init__, 'electrodes'), # required
*get_docval(ElectricalSeries.__init__, 'resolution', 'conversion', 'comments', 'description', 'control',
'control_description', 'offset'))
'control_description', 'offset'),
allow_positional=AllowPositional.WARNING,)
def __init__(self, **kwargs):
data = kwargs['data']
timestamps = kwargs['timestamps']
Expand Down Expand Up @@ -169,7 +172,8 @@ class EventDetection(NWBDataInterface):
'(e.g., .25msec before action potential peak, zero-crossing time, etc). '
'The index points to each event from the raw data'},
{'name': 'times', 'type': ('array_data', 'data'), 'doc': 'Timestamps of events, in Seconds'},
{'name': 'name', 'type': str, 'doc': 'the name of this container', 'default': 'EventDetection'})
{'name': 'name', 'type': str, 'doc': 'the name of this container', 'default': 'EventDetection'},
allow_positional=AllowPositional.WARNING,)
def __init__(self, **kwargs):
args_to_set = popargs_to_dict(('detection_method', 'source_electricalseries', 'source_idx', 'times'), kwargs)
super().__init__(**kwargs)
Expand Down Expand Up @@ -320,7 +324,8 @@ class FeatureExtraction(NWBDataInterface):
'doc': 'The times of events that features correspond to'},
{'name': 'features', 'type': ('array_data', 'data'), 'shape': (None, None, None),
'doc': 'Features for each channel'},
{'name': 'name', 'type': str, 'doc': 'the name of this container', 'default': 'FeatureExtraction'})
{'name': 'name', 'type': str, 'doc': 'the name of this container', 'default': 'FeatureExtraction'},
allow_positional=AllowPositional.WARNING,)
def __init__(self, **kwargs):
# get the inputs
electrodes, description, times, features = popargs(
Expand Down
5 changes: 3 additions & 2 deletions src/pynwb/epoch.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

from hdmf.data_utils import DataIO
from hdmf.common import DynamicTable
from hdmf.utils import docval, getargs, popargs, get_docval
from hdmf.utils import docval, getargs, popargs, get_docval, AllowPositional

from . import register_class, CORE_NAMESPACE
from .base import TimeSeries, TimeSeriesReferenceVectorData, TimeSeriesReference
Expand All @@ -27,7 +27,8 @@ class TimeIntervals(DynamicTable):
@docval({'name': 'name', 'type': str, 'doc': 'name of this TimeIntervals'}, # required
{'name': 'description', 'type': str, 'doc': 'Description of this TimeIntervals',
'default': "experimental intervals"},
*get_docval(DynamicTable.__init__, 'id', 'columns', 'colnames'))
*get_docval(DynamicTable.__init__, 'id', 'columns', 'colnames'),
allow_positional=AllowPositional.WARNING,)
def __init__(self, **kwargs):
super().__init__(**kwargs)

Expand Down
16 changes: 11 additions & 5 deletions src/pynwb/file.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,8 @@ class LabMetaData(NWBContainer):
tutorial.
"""

@docval({'name': 'name', 'type': str, 'doc': 'name of lab metadata'})
@docval({'name': 'name', 'type': str, 'doc': 'name of lab metadata'},
allow_positional=AllowPositional.WARNING,)
def __init__(self, **kwargs):
super().__init__(**kwargs)

Expand Down Expand Up @@ -107,6 +108,7 @@ class Subject(NWBContainer):
{'name': 'date_of_birth', 'type': datetime, 'default': None,
'doc': 'The datetime of the date of birth. May be supplied instead of age.'},
{'name': 'strain', 'type': str, 'doc': 'The strain of the subject, e.g., "C57BL/6J"', 'default': None},
allow_positional=AllowPositional.WARNING,
)
def __init__(self, **kwargs):
keys_to_set = (
Expand Down Expand Up @@ -940,7 +942,8 @@ def get_icephys_simultaneous_recordings(self):
will return None if the table is currently not being used.
"""
if self.icephys_simultaneous_recordings is None:
self.icephys_simultaneous_recordings = SimultaneousRecordingsTable(self.get_intracellular_recordings())
self.icephys_simultaneous_recordings = SimultaneousRecordingsTable(
intracellular_recordings_table=self.get_intracellular_recordings())
return self.icephys_simultaneous_recordings

@docval(*get_docval(SimultaneousRecordingsTable.add_simultaneous_recording),
Expand All @@ -963,7 +966,8 @@ def get_icephys_sequential_recordings(self):
will return None if the table is currently not being used.
"""
if self.icephys_sequential_recordings is None:
self.icephys_sequential_recordings = SequentialRecordingsTable(self.get_icephys_simultaneous_recordings())
self.icephys_sequential_recordings = SequentialRecordingsTable(
simultaneous_recordings_table=self.get_icephys_simultaneous_recordings())
return self.icephys_sequential_recordings

@docval(*get_docval(SequentialRecordingsTable.add_sequential_recording),
Expand All @@ -987,7 +991,8 @@ def get_icephys_repetitions(self):
will return None if the table is currently not being used.
"""
if self.icephys_repetitions is None:
self.icephys_repetitions = RepetitionsTable(self.get_icephys_sequential_recordings())
self.icephys_repetitions = RepetitionsTable(
sequential_recordings_table=self.get_icephys_sequential_recordings())
return self.icephys_repetitions

@docval(*get_docval(RepetitionsTable.add_repetition),
Expand All @@ -1010,7 +1015,8 @@ def get_icephys_experimental_conditions(self):
will return None if the table is currently not being used.
"""
if self.icephys_experimental_conditions is None:
self.icephys_experimental_conditions = ExperimentalConditionsTable(self.get_icephys_repetitions())
self.icephys_experimental_conditions = ExperimentalConditionsTable(
repetitions_table=self.get_icephys_repetitions())
return self.icephys_experimental_conditions

@docval(*get_docval(ExperimentalConditionsTable.add_experimental_condition),
Expand Down
Loading
Loading