Add warnings when using positional arguments in container constructors (

#1972)

* add positional argument warning

* update tests to use kwargs

* update CHANGELOG.md

CHANGELOG.md

@@ -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)
 

src/pynwb/base.py

@@ -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
@@ -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)
@@ -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
         """
@@ -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)
@@ -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)
@@ -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)
@@ -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!

src/pynwb/behavior.py

@@ -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
@@ -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))

src/pynwb/core.py

@@ -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
@@ -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']
@@ -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)

src/pynwb/device.py

@@ -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
@@ -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)

src/pynwb/ecephys.py

@@ -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
@@ -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)
@@ -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)
 
@@ -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']
@@ -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)
@@ -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(

src/pynwb/epoch.py

@@ -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
@@ -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)
 

src/pynwb/file.py

@@ -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)
 
@@ -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 = (
@@ -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),
@@ -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),
@@ -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),
@@ -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),