From e47cd5a55f22dd1d35077a40848580d8d6542c65 Mon Sep 17 00:00:00 2001 From: Steph Prince <40640337+stephprince@users.noreply.github.com> Date: Mon, 23 Dec 2024 10:18:58 -0800 Subject: [PATCH] Update deprecations (#2011) * update deprecated classes and methods * update deprecation warnings * update tests for deprecated classes and functions * fix warnings and formatting * add scratch tests and deprecations * keep function behavior for deprecation warnings * update CHANGELOG.md * add tests for deprecations * update scratch tutorial * skip deprecated gallery examples * update CHANGELOG.md * remove old icephys tutorial * add pynwb 4.0 deprecation info * add error on new pass on construct method * remove warnings on construct for deprecations * fix formatting * update hdmf-zarr url * Apply suggestions from code review Co-authored-by: Ryan Ly * Update src/pynwb/core.py Co-authored-by: Ryan Ly * add code review suggestions * add catch for extra warning * remove unused variable --------- Co-authored-by: Ryan Ly --- CHANGELOG.md | 21 ++ docs/gallery/domain/icephys.py | 239 --------------------- docs/gallery/general/scratch.py | 2 +- docs/source/conf.py | 4 +- src/pynwb/__init__.py | 6 - src/pynwb/base.py | 9 +- src/pynwb/core.py | 36 ++-- src/pynwb/ecephys.py | 8 +- src/pynwb/file.py | 118 ++++------ src/pynwb/icephys.py | 8 +- src/pynwb/image.py | 41 ++-- src/pynwb/ophys.py | 12 +- test.py | 3 +- tests/back_compat/test_import_structure.py | 1 - tests/integration/hdf5/test_ecephys.py | 60 ++++-- tests/integration/hdf5/test_icephys.py | 64 +++--- tests/unit/test_base.py | 28 +-- tests/unit/test_ecephys.py | 42 +++- tests/unit/test_file.py | 44 ++-- tests/unit/test_icephys.py | 80 +++---- tests/unit/test_icephys_metadata_tables.py | 164 +++++++++----- tests/unit/test_image.py | 88 ++++++-- tests/unit/test_ophys.py | 51 +++-- tests/unit/test_scratch.py | 60 ++++++ 24 files changed, 560 insertions(+), 629 deletions(-) delete mode 100644 docs/gallery/domain/icephys.py diff --git a/CHANGELOG.md b/CHANGELOG.md index a7320703f..503df39e6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,27 @@ ## PyNWB 3.0.0 (Upcoming) +### Deprecations +- The following deprecated classes will now raise errors when creating new instances of these classes: ``ClusteringWaveforms``, ``Clustering``, ``SweepTable``. Reading files using these data types will continue to be supported. +- The following methods and arguments have been deprecated: + - ``ProcessingModule.add_container`` and ``ProcessingModule.add_data_interface`` are replaced by ``ProcessingModule.add`` + - ``ProcessingModule.get_container`` and ``ProcessingModule.get_data_interface`` are replaced by ``ProcessingModule.get`` + - ``ScratchData.notes`` is deprecated. Use ``ScratchData.description`` instead. + - ``NWBFile.ic_electrodes`` is deprecated. Use ``NWBFile.icephys_electrodes`` instead. + - ``NWBFile.ec_electrodes`` is deprecated. Use ``NWBFile.electrodes`` instead. + - ``NWBFile.icephys_filtering`` is deprecated. Use ``IntracellularElectrode.filtering`` instead. + - ``NWBFile.modules`` is deprecated. Use ``NWBFile.processing`` instead. + - ``ImageSeries.format`` is fixed to 'external' if an external file is provided. + - ``ImageSeries.bits_per_pixel`` is deprecated. + - ``ImagingPlane.manifold``, ``ImagingPlane.conversion`` and ``ImagingPlane.unit`` are deprecated. Use ``ImagingPlane.origin_coords`` and ``ImagingPlane.grid_spacing`` instead. + - ``IndexSeries.unit`` is fixed to "N\A". + - ``IndexSeries.indexed_timeseries`` is deprecated. Use ``IndexSeries.indexed_images`` instead. +- The following deprecated methods have been removed: + - ``NWBFile.add_ic_electrode`` is removed. Use ``NWBFile.add_icephys_electrode`` instead. + - ``NWBFile.create_ic_electrode`` is removed. Use ``NWBFile.create_icephys_electrode`` instead. + - ``NWBFile.get_ic_electrode`` is removed. Use ``NWBFile.get_icephys_electrode`` instead. + - ``pynwb._get_resources`` is removed. + ### Enhancements and minor changes - Added support for NWB schema 2.8.0. @rly [#2001](https://github.com/NeurodataWithoutBorders/pynwb/pull/2001) - Removed `SpatialSeries.bounds` field that was not functional. This will be fixed in a future release. @rly [#1907](https://github.com/NeurodataWithoutBorders/pynwb/pull/1907), [#1996](https://github.com/NeurodataWithoutBorders/pynwb/pull/1996) diff --git a/docs/gallery/domain/icephys.py b/docs/gallery/domain/icephys.py deleted file mode 100644 index 04eae6bed..000000000 --- a/docs/gallery/domain/icephys.py +++ /dev/null @@ -1,239 +0,0 @@ -# -*- coding: utf-8 -*- -""" -.. _icephys_tutorial: - -Intracellular Electrophysiology Data using SweepTable -===================================================== - -The following tutorial describes storage of intracellular electrophysiology data in NWB using the -SweepTable to manage recordings. - -.. warning:: - The use of SweepTable has been deprecated as of PyNWB >v2.0 in favor of the new hierarchical - intracellular electrophysiology metadata tables to allow for a more complete description of - intracellular electrophysiology experiments. See the :doc:`Intracellular electrophysiology ` - tutorial for details. -""" - -####################### -# Creating and Writing NWB files -# ------------------------------ -# -# When creating a NWB file, the first step is to create the :py:class:`~pynwb.file.NWBFile`. The first -# argument is is a brief description of the dataset. - -# sphinx_gallery_thumbnail_path = 'figures/gallery_thumbnails_icephys_sweeptable.png' -from datetime import datetime -from uuid import uuid4 - -import numpy as np -from dateutil.tz import tzlocal - -from pynwb import NWBFile - -nwbfile = NWBFile( - session_description="my first synthetic recording", - identifier=str(uuid4()), - session_start_time=datetime.now(tzlocal()), - experimenter=[ - "Baggins, Bilbo", - ], - lab="Bag End Laboratory", - institution="University of Middle Earth at the Shire", - experiment_description="I went on an adventure to reclaim vast treasures.", - session_id="LONELYMTN001", -) - -####################### -# Device metadata -# ^^^^^^^^^^^^^^^ -# -# Device metadata is represented by :py:class:`~pynwb.device.Device` objects. -# To create a device, you can use the :py:class:`~pynwb.device.Device` instance method -# :py:meth:`~pynwb.file.NWBFile.create_device`. - -device = nwbfile.create_device(name="Heka ITC-1600") - -####################### -# Electrode metadata -# ^^^^^^^^^^^^^^^^^^ -# -# Intracellular electrode metadata is represented by :py:class:`~pynwb.icephys.IntracellularElectrode` objects. -# To create an electrode group, you can use the :py:class:`~pynwb.file.NWBFile` instance method -# :py:meth:`~pynwb.file.NWBFile.create_icephys_electrode`. - -elec = nwbfile.create_icephys_electrode( - name="elec0", description="a mock intracellular electrode", device=device -) - -####################### -# Stimulus data -# ^^^^^^^^^^^^^ -# -# Intracellular stimulus and response data are represented with subclasses of -# :py:class:`~pynwb.icephys.PatchClampSeries`. There are two classes for representing stimulus -# data -# -# - :py:class:`~pynwb.icephys.VoltageClampStimulusSeries` -# - :py:class:`~pynwb.icephys.CurrentClampStimulusSeries` -# -# and three classes for representing response -# -# - :py:class:`~pynwb.icephys.VoltageClampSeries` -# - :py:class:`~pynwb.icephys.CurrentClampSeries` -# - :py:class:`~pynwb.icephys.IZeroClampSeries` -# -# Here, we will use :py:class:`~pynwb.icephys.CurrentClampStimulusSeries` to store current clamp stimulus -# data and then add it to our NWBFile as stimulus data using the :py:class:`~pynwb.file.NWBFile` method -# :py:meth:`~pynwb.file.NWBFile.add_stimulus`. - -from pynwb.icephys import CurrentClampStimulusSeries - -ccss = CurrentClampStimulusSeries( - name="ccss", - data=[1, 2, 3, 4, 5], - starting_time=123.6, - rate=10e3, - electrode=elec, - gain=0.02, - sweep_number=0, -) - -nwbfile.add_stimulus(ccss, use_sweep_table=True) - -####################### -# We now add another stimulus series but from a different sweep. TimeSeries -# having the same starting time belong to the same sweep. - -from pynwb.icephys import VoltageClampStimulusSeries - -vcss = VoltageClampStimulusSeries( - name="vcss", - data=[2, 3, 4, 5, 6], - starting_time=234.5, - rate=10e3, - electrode=elec, - gain=0.03, - sweep_number=1, -) - -nwbfile.add_stimulus(vcss, use_sweep_table=True) - -####################### -# Here, we will use :py:class:`~pynwb.icephys.CurrentClampSeries` to store current clamp -# data and then add it to our NWBFile as acquired data using the :py:class:`~pynwb.file.NWBFile` method -# :py:meth:`~pynwb.file.NWBFile.add_acquisition`. - -from pynwb.icephys import CurrentClampSeries - -ccs = CurrentClampSeries( - name="ccs", - data=[0.1, 0.2, 0.3, 0.4, 0.5], - conversion=1e-12, - resolution=np.nan, - starting_time=123.6, - rate=20e3, - electrode=elec, - gain=0.02, - bias_current=1e-12, - bridge_balance=70e6, - capacitance_compensation=1e-12, - sweep_number=0, -) - -nwbfile.add_acquisition(ccs, use_sweep_table=True) - -####################### -# And voltage clamp data from the second sweep using -# :py:class:`~pynwb.icephys.VoltageClampSeries`. - -from pynwb.icephys import VoltageClampSeries - -vcs = VoltageClampSeries( - name="vcs", - data=[0.1, 0.2, 0.3, 0.4, 0.5], - conversion=1e-12, - resolution=np.nan, - starting_time=234.5, - rate=20e3, - electrode=elec, - gain=0.02, - capacitance_slow=100e-12, - resistance_comp_correction=70.0, - sweep_number=1, -) - -nwbfile.add_acquisition(vcs, use_sweep_table=True) - -#################### -# .. _icephys_writing: -# -# Once you have finished adding all of your data to the :py:class:`~pynwb.file.NWBFile`, -# write the file with :py:class:`~pynwb.NWBHDF5IO`. - -from pynwb import NWBHDF5IO - -with NWBHDF5IO("icephys_example.nwb", "w") as io: - io.write(nwbfile) - -#################### -# For more details on :py:class:`~pynwb.NWBHDF5IO`, see the :ref:`basic tutorial `. - -#################### -# .. _icephys_reading: -# -# Reading electrophysiology data -# ------------------------------ -# -# Now that you have written some intracellular electrophysiology data, you can read it back in. - -io = NWBHDF5IO("icephys_example.nwb", "r") -nwbfile = io.read() - -#################### -# For details on retrieving data from an :py:class:`~pynwb.file.NWBFile`, we refer the reader to the -# :ref:`basic tutorial `. For this tutorial, we will just get back our the -# :py:class:`~pynwb.icephys.CurrentClampStimulusSeries` object we added above. -# -# First, get the :py:class:`~pynwb.icephys.CurrentClampStimulusSeries` we added as stimulus data. - -ccss = nwbfile.get_stimulus("ccss") - -#################### -# Grabbing acquisition data can be done via :py:meth:`~pynwb.file.NWBFile.get_acquisition` - -vcs = nwbfile.get_acquisition("vcs") - -#################### -# We can also get back the electrode we added. - -elec = nwbfile.get_icephys_electrode("elec0") - -#################### -# Alternatively, we can also get this electrode from the :py:class:`~pynwb.icephys.CurrentClampStimulusSeries` -# we retrieved above. This is exposed via the :py:meth:`~pynwb.icephys.PatchClampSeries.electrode` attribute. - -elec = ccss.electrode - -#################### -# And the device name via :py:meth:`~pynwb.file.NWBFile.get_device` - -device = nwbfile.get_device("Heka ITC-1600") - -#################### -# If you have data from multiple electrodes and multiple sweeps, it can be -# tedious and expensive to search all :py:class:`~pynwb.icephys.PatchClampSeries` for the -# :py:class:`~pynwb.base.TimeSeries` with a given sweep. -# -# Fortunately you don't have to do that manually, instead you can just query -# the :py:class:`~pynwb.icephys.SweepTable` which stores the mapping between the -# PatchClampSeries which belongs to a certain sweep number via -# :py:meth:`~pynwb.icephys.SweepTable.get_series`. -# -# The following call will return the voltage clamp data of two timeseries -# consisting of acquisition and stimulus, from sweep 1. - -series = nwbfile.sweep_table.get_series(1) - -# close the file -io.close() diff --git a/docs/gallery/general/scratch.py b/docs/gallery/general/scratch.py index 0e00c5e96..715f14876 100644 --- a/docs/gallery/general/scratch.py +++ b/docs/gallery/general/scratch.py @@ -145,7 +145,7 @@ # Now lets do an analysis for which we do not have a specification, but we would like to store # the results for. -filt_ts = nwb_scratch.modules["filtering_module"]["filtered_timeseries"] +filt_ts = nwb_scratch.processing["filtering_module"]["filtered_timeseries"] fft = np.fft.fft(filt_ts.data) diff --git a/docs/source/conf.py b/docs/source/conf.py index 7c1f33fa3..c4966c491 100644 --- a/docs/source/conf.py +++ b/docs/source/conf.py @@ -148,7 +148,7 @@ def __call__(self, filename): 'nwbwidgets': ("https://nwb-widgets.readthedocs.io/en/latest/", None), 'nwb-overview': ("https://nwb-overview.readthedocs.io/en/latest/", None), 'zarr': ("https://zarr.readthedocs.io/en/stable/", None), - 'hdmf-zarr': ("https://hdmf-zarr.readthedocs.io/en/latest/", None), + 'hdmf-zarr': ("https://hdmf-zarr.readthedocs.io/en/stable/", None), 'numcodecs': ("https://numcodecs.readthedocs.io/en/latest/", None), } @@ -161,7 +161,7 @@ def __call__(self, filename): 'hdmf-docs': ('https://hdmf.readthedocs.io/en/stable/%s', '%s'), 'dandi': ('https://www.dandiarchive.org/%s', '%s'), "nwbinspector": ("https://nwbinspector.readthedocs.io/en/dev/%s", "%s"), - 'hdmf-zarr': ('https://hdmf-zarr.readthedocs.io/en/latest/%s', '%s'), + 'hdmf-zarr': ('https://hdmf-zarr.readthedocs.io/en/stable/%s', '%s'), } nitpicky = True diff --git a/src/pynwb/__init__.py b/src/pynwb/__init__.py index 32f9ed4d9..5f8fec157 100644 --- a/src/pynwb/__init__.py +++ b/src/pynwb/__init__.py @@ -79,12 +79,6 @@ def __get_resources() -> dict: return ret -def _get_resources(): - # LEGACY: Needed to support legacy implementation. - # TODO: Remove this in PyNWB 3.0. - warn("The function '_get_resources' is deprecated and will be removed in a future release.", DeprecationWarning) - return __get_resources() - # a global type map global __TYPE_MAP diff --git a/src/pynwb/base.py b/src/pynwb/base.py index 0bc5d22d7..bc3aa11b8 100644 --- a/src/pynwb/base.py +++ b/src/pynwb/base.py @@ -51,7 +51,7 @@ def add_container(self, **kwargs): ''' Add an NWBContainer to this ProcessingModule ''' - warn(PendingDeprecationWarning('add_container will be replaced by add')) + warn("add_container is deprecated and will be removed in PyNWB 4.0. Use add instead.", DeprecationWarning) self.add(kwargs['container']) @docval({'name': 'container_name', 'type': str, 'doc': 'the name of the NWBContainer to retrieve'}) @@ -59,18 +59,17 @@ def get_container(self, **kwargs): ''' Retrieve an NWBContainer from this ProcessingModule ''' - warn(PendingDeprecationWarning('get_container will be replaced by get')) + warn('get_container is deprecated and will be removed in PyNWB 4.0. Use get instead.', DeprecationWarning) return self.get(kwargs['container_name']) @docval({'name': 'NWBDataInterface', 'type': (NWBDataInterface, DynamicTable), 'doc': 'the NWBDataInterface to add to this Module'}) def add_data_interface(self, **kwargs): - warn(PendingDeprecationWarning('add_data_interface will be replaced by add')) - self.add(kwargs['NWBDataInterface']) + warn('add_data_interface is deprecated and will be removed in PyNWB 4.0. Use add instead.', DeprecationWarning) @docval({'name': 'data_interface_name', 'type': str, 'doc': 'the name of the NWBContainer to retrieve'}) def get_data_interface(self, **kwargs): - warn(PendingDeprecationWarning('get_data_interface will be replaced by get')) + warn('get_data_interface is deprecated and will be removed in PyNWB 4.0. Use get instead.', DeprecationWarning) return self.get(kwargs['data_interface_name']) diff --git a/src/pynwb/core.py b/src/pynwb/core.py index 59f849db8..332bb5b50 100644 --- a/src/pynwb/core.py +++ b/src/pynwb/core.py @@ -38,15 +38,21 @@ def _error_on_new_warn_on_construct(self, error_msg: str): Raise an error when a check is violated on instance creation. To ensure backwards compatibility, this method throws a warning instead of raising an error when reading from a file, ensuring that - files with invalid data can be read. If error_msg is set to None - the function will simply return without further action. + files with invalid data can be read. """ - if error_msg is None: - return if not self._in_construct_mode: raise ValueError(error_msg) warn(error_msg) + def _error_on_new_pass_on_construct(self, error_msg: str): + """ + Raise an error when a check is violated on instance creation. + When reading from a file, do nothing, ensuring that files with + invalid data or deprecated neurodata types can be read. + """ + if not self._in_construct_mode: + raise ValueError(error_msg) + def _get_type_map(self): return get_type_map() @@ -130,27 +136,29 @@ def __init__(self, **kwargs): notes, description = popargs('notes', 'description', kwargs) super().__init__(**kwargs) if notes != '': - warn('The `notes` argument of ScratchData.__init__ will be deprecated. Use description instead.', - PendingDeprecationWarning) - if notes != '' and description != '': + self._error_on_new_pass_on_construct( + error_msg=("The `notes` argument of ScratchData.__init__ has been deprecated and " + "will be removed in PyNWB 4.0. Use description instead.") + ) + if description is not None: raise ValueError('Cannot provide both notes and description to ScratchData.__init__. The description ' 'argument is recommended.') description = notes if not description: - warn('ScratchData.description will be required in a future major release of PyNWB.', - PendingDeprecationWarning) + self._error_on_new_pass_on_construct(error_msg='ScratchData.description is required.') self.description = description @property def notes(self): - warn('Use of ScratchData.notes will be deprecated. Use ScratchData.description instead.', - PendingDeprecationWarning) + warn(("Use of ScratchData.notes has been deprecated and will be removed in PyNWB 4.0. " + "Use ScratchData.description instead."), DeprecationWarning) return self.description - + @notes.setter def notes(self, value): - warn('Use of ScratchData.notes will be deprecated. Use ScratchData.description instead.', - PendingDeprecationWarning) + self._error_on_new_pass_on_construct( + error_msg=("Use of ScratchData.notes has been deprecated and will be removed in PyNWB 4.0. " + "Use ScratchData.description instead.")) self.description = value diff --git a/src/pynwb/ecephys.py b/src/pynwb/ecephys.py index 41535bbbc..1c5400fe3 100644 --- a/src/pynwb/ecephys.py +++ b/src/pynwb/ecephys.py @@ -231,7 +231,9 @@ class Clustering(NWBDataInterface): 'shape': (None,)}, {'name': 'name', 'type': str, 'doc': 'the name of this container', 'default': 'Clustering'}) def __init__(self, **kwargs): - warnings.warn("use pynwb.misc.Units or NWBFile.units instead", DeprecationWarning) + self._error_on_new_pass_on_construct( + error_msg='The Clustering neurodata type is deprecated. Use pynwb.misc.Units or NWBFile.units instead' + ) args_to_set = popargs_to_dict(('description', 'num', 'peak_over_rms', 'times'), kwargs) super().__init__(**kwargs) args_to_set['peak_over_rms'] = list(args_to_set['peak_over_rms']) @@ -265,7 +267,9 @@ class ClusterWaveforms(NWBDataInterface): 'doc': 'the standard deviations of waveforms for each cluster'}, {'name': 'name', 'type': str, 'doc': 'the name of this container', 'default': 'ClusterWaveforms'}) def __init__(self, **kwargs): - warnings.warn("use pynwb.misc.Units or NWBFile.units instead", DeprecationWarning) + self._error_on_new_pass_on_construct( + error_msg='The ClusterWaveforms neurodata type is deprecated. Use pynwb.misc.Units or NWBFile.units instead' + ) args_to_set = popargs_to_dict(('clustering_interface', 'waveform_filtering', 'waveform_mean', 'waveform_sd'), kwargs) super().__init__(**kwargs) diff --git a/src/pynwb/file.py b/src/pynwb/file.py index 4b4010d4d..27f4acada 100644 --- a/src/pynwb/file.py +++ b/src/pynwb/file.py @@ -384,9 +384,11 @@ class NWBFile(MultiContainerInterface, HERDManager): 'doc': 'the ElectrodeGroups that belong to this NWBFile', 'default': None}, {'name': 'ic_electrodes', 'type': (list, tuple), 'doc': 'DEPRECATED use icephys_electrodes parameter instead. ' - 'IntracellularElectrodes that belong to this NWBFile', 'default': None}, + 'IntracellularElectrodes that belong to this NWBFile', 'default': None}, + # TODO remove this arg in PyNWB 4.0 {'name': 'sweep_table', 'type': SweepTable, - 'doc': 'the SweepTable that belong to this NWBFile', 'default': None}, + 'doc': '[DEPRECATED] Use IntracellularRecordingsTable instead. ' + 'The SweepTable that belong to this NWBFile', 'default': None}, {'name': 'imaging_planes', 'type': (list, tuple), 'doc': 'ImagingPlanes that belong to this NWBFile', 'default': None}, {'name': 'ogen_sites', 'type': (list, tuple), @@ -490,14 +492,20 @@ def __init__(self, **kwargs): args_to_set['file_create_date'] = list(map(_add_missing_timezone, file_create_date)) # backwards-compatibility code for ic_electrodes / icephys_electrodes - icephys_electrodes = args_to_set['icephys_electrodes'] ic_electrodes = args_to_set['ic_electrodes'] - if icephys_electrodes is None and ic_electrodes is not None: - warn("Use of the ic_electrodes parameter is deprecated. " - "Use the icephys_electrodes parameter instead", DeprecationWarning) + if ic_electrodes is not None: + self._error_on_new_pass_on_construct(error_msg=("Use of the ic_electrodes parameter is deprecated " + "and will be removed in PyNWB 4.0. " + "Use the icephys_electrodes parameter instead")) args_to_set['icephys_electrodes'] = ic_electrodes args_to_set.pop('ic_electrodes') # do not set this arg + # backwards-compatibility for sweep table + if args_to_set['sweep_table'] is not None: + self._error_on_new_pass_on_construct(error_msg=("SweepTable is deprecated. Use the " + "IntracellularRecordingsTable instead. See also the " + "NWBFile.add_intracellular_recordings function.")) + # convert single experimenter to tuple experimenter = args_to_set['experimenter'] if isinstance(experimenter, str): @@ -552,30 +560,10 @@ def objects(self): self.all_children() return self.__obj - @property - def modules(self): - warn("NWBFile.modules has been replaced by NWBFile.processing.", DeprecationWarning) - return self.processing - @property def epoch_tags(self): return set(self.epochs.tags[:]) if self.epochs is not None else set() - @property - def ec_electrode_groups(self): - warn("NWBFile.ec_electrode_groups has been replaced by NWBFile.electrode_groups.", DeprecationWarning) - return self.electrode_groups - - @property - def ec_electrodes(self): - warn("NWBFile.ec_electrodes has been replaced by NWBFile.electrodes.", DeprecationWarning) - return self.electrodes - - @property - def ic_electrodes(self): - warn("NWBFile.ic_electrodes has been replaced by NWBFile.icephys_electrodes.", DeprecationWarning) - return self.icephys_electrodes - @property def icephys_filtering(self): return self.fields.get('icephys_filtering') @@ -583,34 +571,11 @@ def icephys_filtering(self): @icephys_filtering.setter def icephys_filtering(self, val): if val is not None: - warn("Use of icephys_filtering is deprecated. Use the IntracellularElectrode.filtering field instead", - DeprecationWarning) + self._error_on_new_warn_on_construct("Use of icephys_filtering is deprecated " + "and will be removed in PyNWB 4.0. " + "Use the IntracellularElectrode.filtering field instead") self.fields['icephys_filtering'] = val - def add_ic_electrode(self, *args, **kwargs): - """ - This method is deprecated and will be removed in future versions. Please - use :py:meth:`~pynwb.file.NWBFile.add_icephys_electrode` instead - """ - warn("NWBFile.add_ic_electrode has been replaced by NWBFile.add_icephys_electrode.", DeprecationWarning) - return self.add_icephys_electrode(*args, **kwargs) - - def create_ic_electrode(self, *args, **kwargs): - """ - This method is deprecated and will be removed in future versions. Please - use :py:meth:`~pynwb.file.NWBFile.create_icephys_electrode` instead - """ - warn("NWBFile.create_ic_electrode has been replaced by NWBFile.create_icephys_electrode.", DeprecationWarning) - return self.create_icephys_electrode(*args, **kwargs) - - def get_ic_electrode(self, *args, **kwargs): - """ - This method is deprecated and will be removed in future versions. Please - use :py:meth:`~pynwb.file.NWBFile.get_icephys_electrode` instead - """ - warn("NWBFile.get_ic_electrode has been replaced by NWBFile.get_icephys_electrode.", DeprecationWarning) - return self.get_icephys_electrode(*args, **kwargs) - def __check_epochs(self): if self.epochs is None: self.epochs = TimeIntervals(name='epochs', description='experimental epochs') @@ -624,13 +589,6 @@ def add_epoch_column(self, **kwargs): self.__check_epochs() self.epochs.add_column(**kwargs) - def add_epoch_metadata_column(self, *args, **kwargs): - """ - This method is deprecated and will be removed in future versions. Please - use :py:meth:`~pynwb.file.NWBFile.add_epoch_column` instead - """ - raise DeprecationWarning("Please use NWBFile.add_epoch_column") - @docval(*get_docval(TimeIntervals.add_interval), allow_extra=True) def add_epoch(self, **kwargs): @@ -839,7 +797,15 @@ def _check_sweep_table(self): Create a SweepTable if not yet done. """ if self.sweep_table is None: - self.sweep_table = SweepTable(name='sweep_table') + if self._in_construct_mode: + # Construct the SweepTable without triggering errors in construct mode because + # SweepTable has been deprecated + sweep_table = SweepTable.__new__(SweepTable, parent=self, in_construct_mode=True) + sweep_table.__init__(name='sweep_table') + sweep_table._in_construct_mode = False + else: + sweep_table = SweepTable(name='sweep_table') + self.sweep_table = sweep_table def _update_sweep_table(self, nwbdata): """ @@ -860,25 +826,11 @@ def add_acquisition(self, **kwargs): if use_sweep_table: self._update_sweep_table(nwbdata) - @docval({'name': 'stimulus', 'type': (TimeSeries, DynamicTable, NWBDataInterface), 'default': None, + @docval({'name': 'stimulus', 'type': (TimeSeries, DynamicTable, NWBDataInterface), 'doc': 'The stimulus presentation data to add to this NWBFile.'}, - {'name': 'use_sweep_table', 'type': bool, 'default': False, 'doc': 'Use the deprecated SweepTable'}, - {'name': 'timeseries', 'type': TimeSeries, 'default': None, - 'doc': 'The "timeseries" keyword argument is deprecated. Use the "nwbdata" argument instead.'},) + {'name': 'use_sweep_table', 'type': bool, 'default': False, 'doc': 'Use the deprecated SweepTable'},) def add_stimulus(self, **kwargs): - stimulus, timeseries = popargs('stimulus', 'timeseries', kwargs) - if stimulus is None and timeseries is None: - raise ValueError( - "The 'stimulus' keyword argument is required. The 'timeseries' keyword argument can be " - "provided for backwards compatibility but is deprecated in favor of 'stimulus' and will be " - "removed in PyNWB 3.0." - ) - # TODO remove this support in PyNWB 3.0 - if timeseries is not None: - warn("The 'timeseries' keyword argument is deprecated and will be removed in PyNWB 3.0. " - "Use the 'stimulus' argument instead.", DeprecationWarning) - if stimulus is None: - stimulus = timeseries + stimulus = popargs('stimulus', kwargs) self._add_stimulus_internal(stimulus) use_sweep_table = popargs('use_sweep_table', kwargs) if use_sweep_table: @@ -1067,12 +1019,14 @@ def get_icephys_meta_parent_table(self): 'doc': 'The name of the data. Required only when passing in a scalar, numpy.ndarray, list, or tuple', 'default': None}, {'name': 'notes', 'type': str, - 'doc': ('Notes to add to the data. Only used when passing in numpy.ndarray, list, or tuple. This ' - 'argument is not recommended. Use the `description` argument instead.'), + 'doc': ('[DEPRECATED] Notes to add to the data. ' + 'Only used when passing in numpy.ndarray, list, or tuple. This argument is not recommended. ' + 'Use the `description` argument instead.'), # TODO remove this arg in PyNWB 4.0 'default': None}, {'name': 'table_description', 'type': str, - 'doc': ('Description for the internal DynamicTable used to store a pandas.DataFrame. This ' + 'doc': ('[DEPRECATED] Description for the internal DynamicTable used to store a pandas.DataFrame. This ' 'argument is not recommended. Use the `description` argument instead.'), + # TODO remove this arg in PyNWB 4.0 'default': ''}, {'name': 'description', 'type': str, 'doc': ('Description of the data. Required only when passing in a scalar, numpy.ndarray, ' @@ -1084,8 +1038,8 @@ def add_scratch(self, **kwargs): data, name, notes, table_description, description = getargs('data', 'name', 'notes', 'table_description', 'description', kwargs) if notes is not None or table_description != '': - warn('Use of the `notes` or `table_description` argument will be removed in a future version of PyNWB. ' - 'Use the `description` argument instead.', PendingDeprecationWarning) + warn(('Use of the `notes` or `table_description` argument is deprecated and will be removed in PyNWB 4.0. ' + 'Use the `description` argument instead.'), DeprecationWarning) if description is not None: raise ValueError('Cannot call add_scratch with (notes or table_description) and description') diff --git a/src/pynwb/icephys.py b/src/pynwb/icephys.py index ef536122c..b8c082535 100644 --- a/src/pynwb/icephys.py +++ b/src/pynwb/icephys.py @@ -345,9 +345,11 @@ class SweepTable(DynamicTable): 'default': "A sweep table groups different PatchClampSeries together."}, *get_docval(DynamicTable.__init__, 'id', 'columns', 'colnames')) def __init__(self, **kwargs): - warnings.warn("Use of SweepTable is deprecated. Use the IntracellularRecordingsTable " - "instead. See also the NWBFile.add_intracellular_recordings function.", - DeprecationWarning) + error_msg=('SweepTable is deprecated. Use the IntracellularRecordingsTable instead. ' + 'See also the NWBFile.add_intracellular_recordings function.') + if not self._in_construct_mode: + raise ValueError(error_msg) + super().__init__(**kwargs) @docval({'name': 'pcs', 'type': PatchClampSeries, diff --git a/src/pynwb/image.py b/src/pynwb/image.py index d5d3e9114..d2ee1c825 100644 --- a/src/pynwb/image.py +++ b/src/pynwb/image.py @@ -96,13 +96,10 @@ def __init__(self, **kwargs): setattr(self, key, val) if self._change_external_file_format(): - warnings.warn( - "%s '%s': The value for 'format' has been changed to 'external'. " - "Setting a default value for 'format' is deprecated and will be changed " - "to raising a ValueError in the next major release." - % (self.__class__.__name__, self.name), - DeprecationWarning, - ) + self._error_on_new_warn_on_construct(error_msg=f"{self.__class__.__name__} '{self.name}': " + "The value for 'format' has been changed to 'external'. " + "If an external file is detected, setting a value for " + "'format' other than 'external' is deprecated.") if not self._check_image_series_dimension(): warnings.warn( @@ -111,13 +108,17 @@ def __init__(self, **kwargs): % (self.__class__.__name__, self.name) ) - self._error_on_new_warn_on_construct( - error_msg=self._check_external_file_starting_frame_length() - ) - self._error_on_new_warn_on_construct( - error_msg=self._check_external_file_format() - ) - self._error_on_new_warn_on_construct(error_msg=self._check_external_file_data()) + error_msg = self._check_external_file_starting_frame_length() + if error_msg: + self._error_on_new_warn_on_construct(error_msg=error_msg) + + error_msg = self._check_external_file_format() + if error_msg: + self._error_on_new_warn_on_construct(error_msg=error_msg) + + error_msg = self._check_external_file_data() + if error_msg: + self._error_on_new_warn_on_construct(error_msg=error_msg) def _change_external_file_format(self): """ @@ -200,7 +201,7 @@ def bits_per_pixel(self): @bits_per_pixel.setter def bits_per_pixel(self, val): if val is not None: - warnings.warn("bits_per_pixel is no longer used", DeprecationWarning) + self._error_on_new_pass_on_construct(error_msg="bits_per_pixel is deprecated") self.fields['bits_per_pixel'] = val @@ -258,16 +259,14 @@ class IndexSeries(TimeSeries): def __init__(self, **kwargs): indexed_timeseries, indexed_images = popargs('indexed_timeseries', 'indexed_images', kwargs) if kwargs['unit'] and kwargs['unit'] != 'N/A': - msg = ("The 'unit' field of IndexSeries is fixed to the value 'N/A' starting in NWB 2.5. Passing " - "a different value for 'unit' will raise an error in PyNWB 3.0.") - warnings.warn(msg, PendingDeprecationWarning) + self._error_on_new_pass_on_construct(error_msg=("The 'unit' field of IndexSeries is " + "fixed to the value 'N/A'.")) if not indexed_timeseries and not indexed_images: msg = "Either indexed_timeseries or indexed_images must be provided when creating an IndexSeries." raise ValueError(msg) if indexed_timeseries: - msg = ("The indexed_timeseries field of IndexSeries is discouraged and will be deprecated in " - "a future version of NWB. Use the indexed_images field instead.") - warnings.warn(msg, PendingDeprecationWarning) + self._error_on_new_pass_on_construct("The indexed_timeseries field of IndexSeries is deprecated. " + "Use the indexed_images field instead.") kwargs['unit'] = 'N/A' # fixed value starting in NWB 2.5 super().__init__(**kwargs) self.indexed_timeseries = indexed_timeseries diff --git a/src/pynwb/ophys.py b/src/pynwb/ophys.py index aecba765d..54c355e70 100644 --- a/src/pynwb/ophys.py +++ b/src/pynwb/ophys.py @@ -115,14 +115,14 @@ def __init__(self, **kwargs): if not isinstance(args_to_set['optical_channel'], list): args_to_set['optical_channel'] = [args_to_set['optical_channel']] if args_to_set['manifold'] is not None: - warnings.warn("The 'manifold' argument is deprecated in favor of 'origin_coords' and 'grid_spacing'.", - DeprecationWarning) + error_msg = "The 'manifold' argument is deprecated in favor of 'origin_coords' and 'grid_spacing'." + self._error_on_new_pass_on_construct(error_msg=error_msg) if args_to_set['conversion'] != 1.0: - warnings.warn("The 'conversion' argument is deprecated in favor of 'origin_coords' and 'grid_spacing'.", - DeprecationWarning) + error_msg = "The 'conversion' argument is deprecated in favor of 'origin_coords' and 'grid_spacing'." + self._error_on_new_pass_on_construct(error_msg=error_msg) if args_to_set['unit'] != 'meters': - warnings.warn("The 'unit' argument is deprecated in favor of 'origin_coords_unit' and 'grid_spacing_unit'.", - DeprecationWarning) + error_msg = "The 'unit' argument is deprecated in favor of 'origin_coords_unit' and 'grid_spacing_unit'." + self._error_on_new_pass_on_construct(error_msg=error_msg) for key, val in args_to_set.items(): setattr(self, key, val) diff --git a/test.py b/test.py index f64fcd75d..de3c2c762 100644 --- a/test.py +++ b/test.py @@ -90,7 +90,6 @@ def _import_from_file(script): os.path.join('domain', 'brain_observatory.py'), # TODO create separate workflow for this ] - def run_example_tests(): """Run the Sphinx gallery example files, excluding ROS3-dependent ones, to check for errors.""" logging.info('running example tests') @@ -99,7 +98,7 @@ def run_example_tests(): for f in files: if f.endswith(".py"): name_with_parent_dir = os.path.join(os.path.basename(root), f) - if name_with_parent_dir in ros3_examples or name_with_parent_dir in allensdk_examples: + if name_with_parent_dir in [*ros3_examples, *allensdk_examples]: logging.info("Skipping %s" % name_with_parent_dir) continue examples_scripts.append(os.path.join(root, f)) diff --git a/tests/back_compat/test_import_structure.py b/tests/back_compat/test_import_structure.py index 81c4acf90..8fde69520 100644 --- a/tests/back_compat/test_import_structure.py +++ b/tests/back_compat/test_import_structure.py @@ -41,7 +41,6 @@ def test_outer_import_structure(self): "__spec__", "__version__", "_due", - "_get_resources", "_version", "available_namespaces", "base", diff --git a/tests/integration/hdf5/test_ecephys.py b/tests/integration/hdf5/test_ecephys.py index a2c0db4b5..9bfdf3086 100644 --- a/tests/integration/hdf5/test_ecephys.py +++ b/tests/integration/hdf5/test_ecephys.py @@ -172,19 +172,31 @@ class TestClusteringIO(AcquisitionH5IOMixin, TestCase): def setUpContainer(self): """ Return a test Clustering to read/write """ - with self.assertWarnsWith(DeprecationWarning, 'use pynwb.misc.Units or NWBFile.units instead'): - return Clustering("A fake Clustering interface", - [0, 1, 2, 0, 1, 2], [100., 101., 102.], [float(i) for i in range(10, 61, 10)]) + # raise error on write + error_msg = "The Clustering neurodata type is deprecated. Use pynwb.misc.Units or NWBFile.units instead" + kwargs = dict(description="A fake Clustering interface", + num=[0, 1, 2, 0, 1, 2], + peak_over_rms=[100., 101., 102.], + times=[float(i) for i in range(10, 61, 10)]) + + # create object with deprecated argument + with self.assertRaisesWith(ValueError, error_msg): + Clustering(**kwargs) + + # create object in construct mode, modeling the behavior of the ObjectMapper on read + # no warning should be raised + obj = Clustering.__new__(Clustering, in_construct_mode=True) + obj.__init__(**kwargs) + + return obj def roundtripContainer(self, cache_spec=False): - # catch the DeprecationWarning raised when reading the Clustering object from file - with self.assertWarnsWith(DeprecationWarning, 'use pynwb.misc.Units or NWBFile.units instead'): - return super().roundtripContainer(cache_spec) + # no warning or error should be raised when reading the Clustering object from file + return super().roundtripContainer(cache_spec) def roundtripExportContainer(self, cache_spec=False): - # catch the DeprecationWarning raised when reading the Clustering object from file - with self.assertWarnsWith(DeprecationWarning, 'use pynwb.misc.Units or NWBFile.units instead'): - return super().roundtripExportContainer(cache_spec) + # no warning or error should be raised when reading the Clustering object from file + return super().roundtripExportContainer(cache_spec) class SpikeEventSeriesConstructor(NWBH5IOFlexMixin, TestCase): @@ -223,12 +235,26 @@ def setUpContainer(self): times = [1.3, 2.3] num = [3, 4] peak_over_rms = [5.3, 6.3] - with self.assertWarnsWith(DeprecationWarning, 'use pynwb.misc.Units or NWBFile.units instead'): - self.clustering = Clustering('description', num, peak_over_rms, times) + + # raise error on write + clust = Clustering.__new__(Clustering, in_construct_mode=True) + clust.__init__('description', num, peak_over_rms, times) + self.clustering = clust + means = [[7.3, 7.3]] stdevs = [[8.3, 8.3]] - with self.assertWarnsWith(DeprecationWarning, 'use pynwb.misc.Units or NWBFile.units instead'): + msg = "The ClusterWaveforms neurodata type is deprecated. Use pynwb.misc.Units or NWBFile.units instead" + with self.assertRaisesWith(ValueError, msg): cw = ClusterWaveforms(self.clustering, 'filtering', means, stdevs) + + # create object in construct mode, modeling the behavior of the ObjectMapper on read + # no warning should be raised + cw = ClusterWaveforms.__new__(ClusterWaveforms, + container_source=None, + parent=None, + in_construct_mode=True) + cw.__init__(self.clustering, 'filtering', means, stdevs) + return cw def addContainer(self, nwbfile): @@ -237,14 +263,12 @@ def addContainer(self, nwbfile): nwbfile.add_acquisition(self.container) def roundtripContainer(self, cache_spec=False): - # catch the DeprecationWarning raised when reading the Clustering object from file - with self.assertWarnsWith(DeprecationWarning, 'use pynwb.misc.Units or NWBFile.units instead'): - return super().roundtripContainer(cache_spec) + # no warning or error should be raised when reading the Clustering object from file + return super().roundtripContainer(cache_spec) def roundtripExportContainer(self, cache_spec=False): - # catch the DeprecationWarning raised when reading the Clustering object from file - with self.assertWarnsWith(DeprecationWarning, 'use pynwb.misc.Units or NWBFile.units instead'): - return super().roundtripExportContainer(cache_spec) + # no warning or error should be raised when reading the Clustering object from file + return super().roundtripExportContainer(cache_spec) class FeatureExtractionConstructor(NWBH5IOFlexMixin, TestCase): diff --git a/tests/integration/hdf5/test_icephys.py b/tests/integration/hdf5/test_icephys.py index b50e5d9f5..c40a51b53 100644 --- a/tests/integration/hdf5/test_icephys.py +++ b/tests/integration/hdf5/test_icephys.py @@ -6,7 +6,6 @@ VoltageClampSeries, IZeroClampSeries) from pynwb.device import Device from pynwb.testing import NWBH5IOMixin, AcquisitionH5IOMixin, TestCase -import warnings class TestIntracellularElectrode(NWBH5IOMixin, TestCase): @@ -153,20 +152,24 @@ def setUpContainer(self): self.pcs = PatchClampSeries(name="pcs", data=[1, 2, 3, 4, 5], unit='A', starting_time=123.6, rate=10e3, electrode=self.elec, gain=0.126, stimulus_description="gotcha ya!", sweep_number=np.uint(4711)) - # Create the SweepTable but ignore the DeprecationWarning - with warnings.catch_warnings(record=True) as w: - warnings.simplefilter('ignore', DeprecationWarning) - sweeptable = SweepTable(name='sweep_table') - # Reissue any other warnings that may have occurred - for i in w: - warnings.warn(i.message, i.category) + + # create the sweeptable in construct mode, modeling the behavior of the ObjectMapper on read + # no user warning or error should be raised + sweeptable = SweepTable.__new__(SweepTable) + sweeptable._in_construct_mode = True + sweeptable.__init__(name='sweep_table') + sweeptable._in_construct_mode = False + return sweeptable def addContainer(self, nwbfile): """ Add the test SweepTable, PatchClampSeries, IntracellularElectrode, and Device to the given NWBFile """ - nwbfile.sweep_table = self.container + # NOTE - if the SweepTable creation warning has already been bypassed so that self.container is an + # instance of SweepTable, then adding sweep_table to the NWBFile in this way will not trigger an + # error + nwbfile.sweep_table = self.container nwbfile.add_device(self.device) nwbfile.add_icephys_electrode(self.elec) nwbfile.add_acquisition(self.pcs, use_sweep_table=True) @@ -177,17 +180,13 @@ def getContainer(self, nwbfile): def roundtripContainer(self, cache_spec=False): # catch the DeprecationWarning raised when reading the SweepTable object from file - with self.assertWarnsWith(DeprecationWarning, - "Use of SweepTable is deprecated. Use the IntracellularRecordingsTable instead. " - "See also the NWBFile.add_intracellular_recordings function."): - return super().roundtripContainer(cache_spec) + # no warning or error message should be raised + return super().roundtripContainer(cache_spec) def roundtripExportContainer(self, cache_spec=False): # catch the DeprecationWarning raised when reading the SweepTable object from file - with self.assertWarnsWith(DeprecationWarning, - "Use of SweepTable is deprecated. Use the IntracellularRecordingsTable instead. " - "See also the NWBFile.add_intracellular_recordings function."): - return super().roundtripExportContainer(cache_spec) + # no warning or error message should be raised + return super().roundtripExportContainer(cache_spec) def test_container(self): """ Test properties of the SweepTable read from file """ @@ -224,19 +223,22 @@ def setUpContainer(self): starting_time=123.6, rate=10e3, electrode=self.elec, gain=0.126, stimulus_description="gotcha ya!", sweep_number=np.uint(4712)) - # Create the SweepTable but ignore the DeprecationWarning - with warnings.catch_warnings(record=True) as w: - warnings.simplefilter('ignore', DeprecationWarning) - sweeptable = SweepTable(name='sweep_table') - # Reissue any other warnings that may have occurred - for i in w: - warnings.warn(i.message, i.category) + # create the sweeptable in construct mode, modeling the behavior of the ObjectMapper on read + # no warning or error should be raised + sweeptable = SweepTable.__new__(SweepTable) + sweeptable._in_construct_mode = True + sweeptable.__init__(name='sweep_table') + sweeptable._in_construct_mode = False + return sweeptable def addContainer(self, nwbfile): """ Add the test SweepTable, PatchClampSeries, IntracellularElectrode, and Device to the given NWBFile """ + # NOTE - if the SweepTable creation error has already been bypassed so that self.container is an + # instance of SweepTable, then adding sweep_table to the NWBFile in this way will not trigger an + # error nwbfile.sweep_table = self.container nwbfile.add_device(self.device) nwbfile.add_icephys_electrode(self.elec) @@ -250,18 +252,12 @@ def getContainer(self, nwbfile): return nwbfile.sweep_table def roundtripContainer(self, cache_spec=False): - # catch the DeprecationWarning raised when reading the SweepTable object from file - with self.assertWarnsWith(DeprecationWarning, - "Use of SweepTable is deprecated. Use the IntracellularRecordingsTable instead. " - "See also the NWBFile.add_intracellular_recordings function."): - return super().roundtripContainer(cache_spec) + # no warning or error should be raised when reading the SweepTable object from file + return super().roundtripContainer(cache_spec) def roundtripExportContainer(self, cache_spec=False): - # catch the DeprecationWarning raised when reading the SweepTable object from file - with self.assertWarnsWith(DeprecationWarning, - "Use of SweepTable is deprecated. Use the IntracellularRecordingsTable instead. " - "See also the NWBFile.add_intracellular_recordings function."): - return super().roundtripExportContainer(cache_spec) + # no warning or error should be raised when reading the SweepTable object from file + return super().roundtripExportContainer(cache_spec) def test_container(self): """ Test properties of the SweepTable read from file """ diff --git a/tests/unit/test_base.py b/tests/unit/test_base.py index 5af4986ac..a1afe1d15 100644 --- a/tests/unit/test_base.py +++ b/tests/unit/test_base.py @@ -45,21 +45,19 @@ def test_add_data_interface(self): def test_deprecated_add_data_interface(self): ts = self._create_time_series() - with self.assertWarnsWith( - PendingDeprecationWarning, "add_data_interface will be replaced by add" + msg = 'add_data_interface is deprecated and will be removed in PyNWB 4.0. Use add instead.' + with self.assertWarnsWith(warn_type=DeprecationWarning, + exc_msg=msg ): self.pm.add_data_interface(ts) - self.assertIn(ts.name, self.pm.containers) - self.assertIs(ts, self.pm.containers[ts.name]) def test_deprecated_add_container(self): ts = self._create_time_series() - with self.assertWarnsWith( - PendingDeprecationWarning, "add_container will be replaced by add" + msg = 'add_container is deprecated and will be removed in PyNWB 4.0. Use add instead.' + with self.assertWarnsWith(warn_type=DeprecationWarning, + exc_msg=msg ): self.pm.add_container(ts) - self.assertIn(ts.name, self.pm.containers) - self.assertIs(ts, self.pm.containers[ts.name]) def test_get_data_interface(self): """Test adding a data interface to a ProcessingModule and retrieving it using get(...).""" @@ -72,20 +70,22 @@ def test_get_data_interface(self): def test_deprecated_get_data_interface(self): ts = self._create_time_series() self.pm.add(ts) - with self.assertWarnsWith( - PendingDeprecationWarning, "get_data_interface will be replaced by get" + msg = 'get_data_interface is deprecated and will be removed in PyNWB 4.0. Use get instead.' + with self.assertWarnsWith(warn_type=DeprecationWarning, + exc_msg=msg ): tmp = self.pm.get_data_interface("test_ts") - self.assertIs(tmp, ts) + self.assertIs(tmp, ts) def test_deprecated_get_container(self): ts = self._create_time_series() self.pm.add(ts) - with self.assertWarnsWith( - PendingDeprecationWarning, "get_container will be replaced by get" + msg = 'get_container is deprecated and will be removed in PyNWB 4.0. Use get instead.' + with self.assertWarnsWith(warn_type=DeprecationWarning, + exc_msg=msg ): tmp = self.pm.get_container("test_ts") - self.assertIs(tmp, ts) + self.assertIs(tmp, ts) def test_getitem(self): """Test adding a data interface to a ProcessingModule and retrieving it using __getitem__(...).""" diff --git a/tests/unit/test_ecephys.py b/tests/unit/test_ecephys.py index d70953c12..362e0910a 100644 --- a/tests/unit/test_ecephys.py +++ b/tests/unit/test_ecephys.py @@ -290,8 +290,19 @@ def test_init(self): num = [3, 4] peak_over_rms = [5.3, 6.3] - with self.assertWarnsWith(DeprecationWarning, 'use pynwb.misc.Units or NWBFile.units instead'): - cc = Clustering(description='description', num=num, peak_over_rms=peak_over_rms, times=times) + error_msg = "The Clustering neurodata type is deprecated. Use pynwb.misc.Units or NWBFile.units instead" + kwargs = dict(description='description', + num=num, + peak_over_rms=peak_over_rms, + times=times) + with self.assertRaisesWith(ValueError, error_msg): + cc = Clustering(**kwargs) + + # create object in construct mode, modeling the behavior of the ObjectMapper on read + # no error or warning should be raised + cc = Clustering.__new__(Clustering, in_construct_mode=True) + cc.__init__(**kwargs) + self.assertEqual(cc.description, 'description') self.assertEqual(cc.num, num) self.assertEqual(cc.peak_over_rms, peak_over_rms) @@ -304,19 +315,28 @@ def test_init(self): times = [1.3, 2.3] num = [3, 4] peak_over_rms = [5.3, 6.3] - with self.assertWarnsWith(DeprecationWarning, 'use pynwb.misc.Units or NWBFile.units instead'): - cc = Clustering(description='description', num=num, peak_over_rms=peak_over_rms, times=times) + + # create object in construct mode, modeling the behavior of the ObjectMapper on read + cc = Clustering.__new__(Clustering, + container_source=None, + parent=None, + in_construct_mode=True) + cc.__init__('description', num, peak_over_rms, times) means = [[7.3, 7.3]] stdevs = [[8.3, 8.3]] + error_msg = "The ClusterWaveforms neurodata type is deprecated. Use pynwb.misc.Units or NWBFile.units instead" + with self.assertRaisesWith(ValueError, error_msg): + cw = ClusterWaveforms(cc, 'filtering', means, stdevs) + + # create object in construct mode, modeling the behavior of the ObjectMapper on read + # no error or warning should be raised + cw = ClusterWaveforms.__new__(ClusterWaveforms, + container_source=None, + parent=None, + in_construct_mode=True) + cw.__init__(cc, 'filtering', means, stdevs) - with self.assertWarnsWith(DeprecationWarning, 'use pynwb.misc.Units or NWBFile.units instead'): - cw = ClusterWaveforms( - clustering_interface=cc, - waveform_filtering='filtering', - waveform_mean=means, - waveform_sd=stdevs - ) self.assertEqual(cw.clustering_interface, cc) self.assertEqual(cw.waveform_filtering, 'filtering') self.assertEqual(cw.waveform_mean, means) diff --git a/tests/unit/test_file.py b/tests/unit/test_file.py index 9465a6f23..c76abd7f5 100644 --- a/tests/unit/test_file.py +++ b/tests/unit/test_file.py @@ -126,12 +126,13 @@ def test_access_group_after_io(self): remove_test_file("electrodes_mwe.nwb") - def test_access_processing(self): + def test_access_processing_with_modules(self): self.nwbfile.create_processing_module('test_mod', 'test_description') - # test deprecate .modules - with self.assertWarnsWith(DeprecationWarning, 'NWBFile.modules has been replaced by NWBFile.processing.'): - modules = self.nwbfile.modules['test_mod'] - self.assertIs(self.nwbfile.processing['test_mod'], modules) + + # create object with deprecated argument + msg = "'NWBFile' object has no attribute 'modules'" + with self.assertRaisesWith(AttributeError, msg): + self.nwbfile.modules['test_mod'] def test_epoch_tags(self): tags1 = ['t1', 't2'] @@ -175,11 +176,8 @@ def test_add_stimulus(self): def test_add_stimulus_timeseries_arg(self): """Test nwbfile.add_stimulus using the deprecated 'timeseries' keyword argument""" - msg = ( - "The 'timeseries' keyword argument is deprecated and will be removed in PyNWB 3.0. " - "Use the 'stimulus' argument instead." - ) - with self.assertWarnsWith(DeprecationWarning, msg): + msg = ("NWBFile.add_stimulus: missing argument 'stimulus', unrecognized argument: 'timeseries'") + with self.assertRaisesWith(TypeError, msg): self.nwbfile.add_stimulus( timeseries=TimeSeries( name='test_ts', @@ -188,17 +186,12 @@ def test_add_stimulus_timeseries_arg(self): timestamps=[0.0, 0.1, 0.2, 0.3, 0.4, 0.5] ) ) - self.assertEqual(len(self.nwbfile.stimulus), 1) def test_add_stimulus_no_stimulus_arg(self): """Test nwbfile.add_stimulus using the deprecated 'timeseries' keyword argument""" - msg = ( - "The 'stimulus' keyword argument is required. The 'timeseries' keyword argument can be " - "provided for backwards compatibility but is deprecated in favor of 'stimulus' and will be " - "removed in PyNWB 3.0." - ) - with self.assertRaisesWith(ValueError, msg): - self.nwbfile.add_stimulus(None) + msg = ("NWBFile.add_stimulus: missing argument 'stimulus'") + with self.assertRaisesWith(TypeError, msg): + self.nwbfile.add_stimulus() self.assertEqual(len(self.nwbfile.stimulus), 0) def test_add_stimulus_dynamic_table(self): @@ -542,6 +535,21 @@ def test_multi_publications(self): related_publications=('pub1', 'pub2')) self.assertTupleEqual(self.nwbfile.related_publications, ('pub1', 'pub2')) + def test_ec_electrodes_deprecation(self): + nwbfile = NWBFile('a', 'b', datetime.now(tzlocal())) + device = nwbfile.create_device('a') + elecgrp = nwbfile.create_electrode_group('name', 'desc', device=device, location='a') + nwbfile.add_electrode(location='loc1', group=elecgrp, id=0) + + # test that NWBFile.ec_electrodes property warns or errors + msg = "'NWBFile' object has no attribute 'ec_electrodes'" + with self.assertRaisesWith(AttributeError, msg): + nwbfile.ec_electrodes + + # test that NWBFile.ec_electrode_groups warns or errors + msg = "'NWBFile' object has no attribute 'ec_electrode_groups'" + with self.assertRaisesWith(AttributeError, msg): + nwbfile.ec_electrode_groups class SubjectTest(TestCase): def setUp(self): diff --git a/tests/unit/test_icephys.py b/tests/unit/test_icephys.py index 642aa5411..78cf3eb50 100644 --- a/tests/unit/test_icephys.py +++ b/tests/unit/test_icephys.py @@ -39,26 +39,31 @@ class NWBFileICEphys(TestCase): def setUp(self): self.icephys_electrode = GetElectrode() - def test_sweep_table_depractation_warn(self): - msg = ("Use of SweepTable is deprecated. Use the IntracellularRecordingsTable " - "instead. See also the NWBFile.add_intracellular_recordings function.") - with self.assertWarnsWith(DeprecationWarning, msg): - _ = NWBFile( - session_description='NWBFile icephys test', - identifier='NWB123', # required - session_start_time=datetime(2017, 4, 3, 11, tzinfo=tzlocal()), - ic_electrodes=[self.icephys_electrode, ], - sweep_table=SweepTable()) - - def test_ic_electrodes_parameter_deprecation(self): - # Make sure we warn when using the ic_electrodes parameter on NWBFile - msg = "Use of the ic_electrodes parameter is deprecated. Use the icephys_electrodes parameter instead" - with self.assertWarnsWith(DeprecationWarning, msg): - _ = NWBFile( - session_description='NWBFile icephys test', + def test_sweep_table_deprecation_warn(self): + msg = ("SweepTable is deprecated. Use the IntracellularRecordingsTable instead. " + "See also the NWBFile.add_intracellular_recordings function.") + + with self.assertRaisesWith(ValueError, msg): + SweepTable() + + # create object in construct mode, modeling the behavior of the ObjectMapper on read + # should not raise error or warning + sweepT = SweepTable.__new__(SweepTable, in_construct_mode=True) + sweepT.__init__() + + kwargs = dict(session_description='NWBFile icephys test', identifier='NWB123', # required session_start_time=datetime(2017, 4, 3, 11, tzinfo=tzlocal()), - ic_electrodes=[self.icephys_electrode, ]) + icephys_electrodes=[self.icephys_electrode, ], + sweep_table=sweepT) + + with self.assertRaisesWith(ValueError, msg): + NWBFile(**kwargs) + + # create object in construct mode, modeling the behavior of the ObjectMapper on read + # should not warn or error + nwbfile = NWBFile.__new__(NWBFile, in_construct_mode=True) + nwbfile.__init__(**kwargs) def test_icephys_electrodes_parameter(self): nwbfile = NWBFile( @@ -68,53 +73,18 @@ def test_icephys_electrodes_parameter(self): icephys_electrodes=[self.icephys_electrode, ]) self.assertEqual(nwbfile.get_icephys_electrode('test_iS'), self.icephys_electrode) - def test_add_ic_electrode_deprecation(self): - # Make sure we warn when using the add_ic_electrodes parameter on NWBFile - nwbfile = NWBFile( - session_description='NWBFile icephys test', - identifier='NWB123', # required - session_start_time=datetime(2017, 4, 3, 11, tzinfo=tzlocal())) - - msg = "NWBFile.add_ic_electrode has been replaced by NWBFile.add_icephys_electrode." - with self.assertWarnsWith(DeprecationWarning, msg): - nwbfile.add_ic_electrode(self.icephys_electrode) - def test_ic_electrodes_attribute_deprecation(self): nwbfile = NWBFile( session_description='NWBFile icephys test', identifier='NWB123', # required session_start_time=datetime(2017, 4, 3, 11, tzinfo=tzlocal()), icephys_electrodes=[self.icephys_electrode, ]) - # make sure NWBFile.ic_electrodes property warns - - msg = "NWBFile.ic_electrodes has been replaced by NWBFile.icephys_electrodes." - with self.assertWarnsWith(DeprecationWarning, msg): - nwbfile.ic_electrodes # make sure NWBFile.get_ic_electrode warns - msg = "NWBFile.get_ic_electrode has been replaced by NWBFile.get_icephys_electrode." - with self.assertWarnsWith(DeprecationWarning, msg): + msg = "'NWBFile' object has no attribute 'get_ic_electrode'" + with self.assertRaisesWith(AttributeError, msg): nwbfile.get_ic_electrode(self.icephys_electrode.name) - def test_create_ic_electrode_deprecation(self): - nwbfile = NWBFile( - session_description='NWBFile icephys test', - identifier='NWB123', # required - session_start_time=datetime(2017, 4, 3, 11, tzinfo=tzlocal())) - device = Device(name='device_name') - msg = "NWBFile.create_ic_electrode has been replaced by NWBFile.create_icephys_electrode." - with self.assertWarnsWith(DeprecationWarning, msg): - nwbfile.create_ic_electrode( - name='test_iS', - device=device, - description='description', - slice='slice', - seal='seal', - location='location', - resistance='resistance', - filtering='filtering', - initial_access_resistance='initial_access_resistance') - class IntracellularElectrodeConstructor(TestCase): diff --git a/tests/unit/test_icephys_metadata_tables.py b/tests/unit/test_icephys_metadata_tables.py index b6ccf4c90..705c91f5d 100644 --- a/tests/unit/test_icephys_metadata_tables.py +++ b/tests/unit/test_icephys_metadata_tables.py @@ -1102,15 +1102,28 @@ def test_deprecate_simultaneous_recordings_on_add_stimulus(self): electrode = self.__add_electrode(nwbfile, device) stimulus = self.__get_stimulus(electrode=electrode) response = self.__get_response(electrode=electrode) - # Make sure we warn if sweeptable is added on add_stimulus - with warnings.catch_warnings(record=True) as w: - warnings.simplefilter("always") # Trigger all warnings + stimulus2 = VoltageClampStimulusSeries( + name="ccss2", + data=[1, 2, 3, 4, 5], + starting_time=123.6, + rate=10e3, + electrode=electrode, + gain=0.02, + sweep_number=np.uint64(16) + ) + + msg = ("SweepTable is deprecated. Use the IntracellularRecordingsTable instead. " + "See also the NWBFile.add_intracellular_recordings function.") + with self.assertRaisesWith(ValueError, msg): nwbfile.add_stimulus(stimulus, use_sweep_table=True) - self.assertEqual(len(w), 1) - assert issubclass(w[-1].category, DeprecationWarning) - # make sure we don't trigger the same deprecation warning twice - nwbfile.add_acquisition(response, use_sweep_table=True) - self.assertEqual(len(w), 1) + + # test that adding a stimulus does not error when in construct mode + nwbfile._in_construct_mode = True + nwbfile.add_stimulus(stimulus2, use_sweep_table=True) + + # make sure we don't trigger the same deprecation warning twice + nwbfile.add_acquisition(response, use_sweep_table=True) + nwbfile._in_construct_mode = False def test_deprecate_sweeptable_on_add_stimulus_template(self): """ @@ -1140,16 +1153,18 @@ def test_deprecate_sweeptable_on_add_stimulus_template(self): gain=0.02, sweep_number=np.uint64(15) ) - with warnings.catch_warnings(record=True) as w: + + msg = ("SweepTable is deprecated. Use the IntracellularRecordingsTable instead. " + "See also the NWBFile.add_intracellular_recordings function.") + with self.assertRaisesWith(ValueError, msg): nwbfile.add_stimulus_template(local_stimulus, use_sweep_table=True) - self.assertEqual(len(w), 1) - assert issubclass(w[-1].category, DeprecationWarning) - self.assertEqual(str(w[-1].message), - "Use of SweepTable is deprecated. Use the IntracellularRecordingsTable " - "instead. See also the NWBFile.add_intracellular_recordings function.") - # make sure we don't trigger the same deprecation warning twice - nwbfile.add_stimulus_template(local_stimulus2, use_sweep_table=True) - self.assertEqual(len(w), 1) + # NOTE - the sweep table creation will error but the stimulus template will still be added + + # create object in construct mode, modeling the behavior of the ObjectMapper on read + # should not trigger any warnings or errors + nwbfile._in_construct_mode = True + nwbfile.add_stimulus_template(local_stimulus2, use_sweep_table=True) + nwbfile._in_construct_mode = False def test_deprecate_sweepstable_on_add_acquistion(self): """ @@ -1160,49 +1175,84 @@ def test_deprecate_sweepstable_on_add_acquistion(self): electrode = self.__add_electrode(nwbfile, device) stimulus = self.__get_stimulus(electrode=electrode) response = self.__get_response(electrode=electrode) - # Make sure we warn if sweeptable is added on add_stimulus - with warnings.catch_warnings(record=True) as w: + response2 = VoltageClampSeries( + name='vcs2', + data=[0.1, 0.2, 0.3, 0.4, 0.5], + conversion=1e-12, + resolution=np.nan, + starting_time=123.6, + rate=20e3, + electrode=electrode, + gain=0.02, + capacitance_slow=100e-12, + resistance_comp_correction=70.0, + sweep_number=np.uint64(16) + ) + + msg = ("SweepTable is deprecated. Use the IntracellularRecordingsTable instead. " + "See also the NWBFile.add_intracellular_recordings function.") + + # check we raise an error when using the sweeptable with add_acquisition + with self.assertRaisesWith(ValueError, msg): nwbfile.add_acquisition(response, use_sweep_table=True) - self.assertEqual(len(w), 1) - assert issubclass(w[-1].category, DeprecationWarning) - self.assertEqual(str(w[-1].message), - "Use of SweepTable is deprecated. Use the IntracellularRecordingsTable " - "instead. See also the NWBFile.add_intracellular_recordings function.") - # make sure we don't trigger the same deprecation warning twice - nwbfile.add_stimulus(stimulus, use_sweep_table=True) - self.assertEqual(len(w), 1) + # NOTE - the sweep table creation will error but the acquisition will still be added + + # should not trigger error or warning in construct mode + nwbfile._in_construct_mode = True + nwbfile.add_acquisition(response2, use_sweep_table=True) + + # make sure we don't trigger the same deprecation warning twice + nwbfile.add_stimulus(stimulus, use_sweep_table=True) + nwbfile._in_construct_mode = False + def test_deprecate_sweepstable_on_init(self): """ Test that warnings are raised if the user tries to use a sweeps table """ from pynwb.icephys import SweepTable - with warnings.catch_warnings(record=True) as w: - nwbfile = NWBFile( - session_description='my first synthetic recording', + sweepT = SweepTable.__new__(SweepTable, in_construct_mode=True) + sweepT.__init__() + + kwargs = dict(session_description='my first synthetic recording', identifier='EXAMPLE_ID', session_start_time=datetime.now(tzlocal()), - sweep_table=SweepTable() - ) - device = self.__add_device(nwbfile) - electrode = self.__add_electrode(nwbfile, device) - stimulus = self.__get_stimulus(electrode=electrode) - self.assertEqual(len(w), 1) - assert issubclass(w[-1].category, DeprecationWarning) - # make sure we don't trigger the same deprecation warning twice - nwbfile.add_stimulus(stimulus, use_sweep_table=True) - self.assertEqual(len(w), 1) + sweep_table=sweepT) + + # check we raise an error when using the sweeptable argument on init + msg = ("SweepTable is deprecated. Use the IntracellularRecordingsTable instead. " + "See also the NWBFile.add_intracellular_recordings function.") + with self.assertRaisesWith(ValueError, msg): + nwbfile = NWBFile(**kwargs) + + # create object in construct mode, modeling the behavior of the ObjectMapper on read + # should not trigger warning or error + nwbfile = NWBFile.__new__(NWBFile, in_construct_mode=True) + nwbfile.__init__(**kwargs) + + # make sure we don't trigger the same deprecation warning twice + device = self.__add_device(nwbfile) + electrode = self.__add_electrode(nwbfile, device) + stimulus = self.__get_stimulus(electrode=electrode) + nwbfile.add_stimulus(stimulus, use_sweep_table=True) - def test_deprectation_icephys_filtering_on_init(self): - with warnings.catch_warnings(record=True) as w: - nwbfile = NWBFile( - session_description='my first synthetic recording', + def test_deprecation_icephys_filtering_on_init(self): + kwargs = dict(session_description='my first synthetic recording', identifier='EXAMPLE_ID', session_start_time=datetime.now(tzlocal()), - icephys_filtering='test filtering' - ) - assert issubclass(w[-1].category, DeprecationWarning) - self.assertEqual(nwbfile.icephys_filtering, 'test filtering') + icephys_filtering='test filtering') + msg = ("Use of icephys_filtering is deprecated and will be removed in PyNWB 4.0. " + "Use the IntracellularElectrode.filtering field instead") + + with self.assertRaisesWith(ValueError, msg): + nwbfile = NWBFile(**kwargs) + + # create object in construct mode, modeling the behavior of the ObjectMapper on read + nwbfile = NWBFile.__new__(NWBFile, in_construct_mode=True) + with self.assertWarnsWith(warn_type=UserWarning, exc_msg=msg): + nwbfile.__init__(**kwargs) + + self.assertEqual(nwbfile.icephys_filtering, 'test filtering') def test_icephys_filtering_roundtrip(self): # create the base file @@ -1212,18 +1262,24 @@ def test_icephys_filtering_roundtrip(self): session_start_time=datetime.now(tzlocal()) ) # set the icephys_filtering attribute and make sure we get a deprecation warning - with warnings.catch_warnings(record=True) as w: + msg = ("Use of icephys_filtering is deprecated and will be removed in PyNWB 4.0. " + "Use the IntracellularElectrode.filtering field instead") + with self.assertRaisesWith(ValueError, msg): + nwbfile.icephys_filtering = 'test filtering' + + # create object in construct mode, modeling the behavior of the ObjectMapper on read + nwbfile._in_construct_mode = True + with self.assertWarnsWith(warn_type=UserWarning, exc_msg=msg): nwbfile.icephys_filtering = 'test filtering' - assert issubclass(w[-1].category, DeprecationWarning) - # write the test fil + nwbfile._in_construct_mode = False + + # write the test file with NWBHDF5IO(self.path, 'w') as io: io.write(nwbfile) # read the test file and confirm icephys_filtering has been written with NWBHDF5IO(self.path, 'r') as io: - with warnings.catch_warnings(record=True) as w: + with self.assertWarnsWith(UserWarning, msg): infile = io.read() - self.assertEqual(len(w), 1) # make sure a warning is being raised - assert issubclass(w[0].category, DeprecationWarning) # make sure it is a deprecation warning self.assertEqual(infile.icephys_filtering, 'test filtering') # make sure the value is set def test_get_icephys_meta_parent_table(self): diff --git a/tests/unit/test_image.py b/tests/unit/test_image.py index e41475f8f..11a2b7a8d 100644 --- a/tests/unit/test_image.py +++ b/tests/unit/test_image.py @@ -154,7 +154,7 @@ def test_external_file_with_incorrect_starting_frame_construct_mode(self): "ImageSeries 'test_iS': The number of frame indices in " "'starting_frame' should have the same length as 'external_file'." ) - # Create the image series in construct mode, modelling the behavior + # Create the image series in construct mode, modeling the behavior # of the ObjectMapper on read while avoiding having to create, write, # and read and entire NWB file obj = ImageSeries.__new__(ImageSeries, @@ -243,17 +243,23 @@ def test_external_file_default_format(self): """Test that format is set to 'external' if not provided, when external_file is provided.""" msg = ( "ImageSeries 'test_iS': The value for 'format' has been changed to 'external'. " - "Setting a default value for 'format' is deprecated and will be changed to " - "raising a ValueError in the next major release." + "If an external file is detected, setting a value for " + "'format' other than 'external' is deprecated." ) - with self.assertWarnsWith(DeprecationWarning, msg): - iS = ImageSeries( - name="test_iS", + kwargs = dict(name="test_iS", external_file=["external_file", "external_file2"], unit="n.a.", starting_frame=[0, 10], - rate=0.2, - ) + rate=0.2,) + + # create object with deprecated argument + with self.assertRaisesWith(ValueError, msg): + ImageSeries(**kwargs) + + # create object in construct mode, modeling the behavior of the ObjectMapper on read + iS = ImageSeries.__new__(ImageSeries, in_construct_mode=True) + with self.assertWarnsWith(warn_type=UserWarning, exc_msg=msg): + iS.__init__(**kwargs) self.assertEqual(iS.format, "external") def test_external_file_with_correct_format(self): @@ -309,6 +315,23 @@ def test_external_file_with_data_construct_mode(self): rate=0.2, ) + def test_bits_per_pixel_deprecation(self): + msg = "bits_per_pixel is deprecated" + kwargs = dict(name='test_iS', + unit='unit', + external_file=['external_file'], + starting_frame=[0], + format='external', + timestamps=[1., 2., 3.], + bits_per_pixel=8) + with self.assertRaisesWith(ValueError, msg): + ImageSeries(**kwargs) + + # create object in construct mode, modeling the behavior of the ObjectMapper on read + # no warning or error should be raised + iS = ImageSeries.__new__(ImageSeries, in_construct_mode=True) + iS.__init__(**kwargs) + class IndexSeriesConstructor(TestCase): @@ -329,23 +352,29 @@ def test_init(self): self.assertEqual(iS.unit, 'N/A') self.assertIs(iS.indexed_images, images) - def test_init_bad_unit(self): + def test_init_bad_unit(self): ts = TimeSeries( name='test_ts', data=[1, 2, 3], unit='unit', timestamps=[0.1, 0.2, 0.3] ) - msg = ("The 'unit' field of IndexSeries is fixed to the value 'N/A' starting in NWB 2.5. Passing " - "a different value for 'unit' will raise an error in PyNWB 3.0.") - with self.assertWarnsWith(PendingDeprecationWarning, msg): - iS = IndexSeries( - name='test_iS', + msg = ("The 'unit' field of IndexSeries is fixed to the value 'N/A'.") + kwargs = dict(name='test_iS', data=[1, 2, 3], unit='na', indexed_timeseries=ts, - timestamps=[0.1, 0.2, 0.3] - ) + timestamps=[0.1, 0.2, 0.3]) + + # create object with deprecated argument + with self.assertRaisesWith(ValueError, msg): + IndexSeries(**kwargs) + + # create object in construct mode, modeling the behavior of the ObjectMapper on read + # no warning or error should be raised + iS = IndexSeries.__new__(IndexSeries, in_construct_mode=True) + iS.__init__(**kwargs) + self.assertEqual(iS.unit, 'N/A') def test_init_indexed_ts(self): @@ -355,18 +384,31 @@ def test_init_indexed_ts(self): unit='unit', timestamps=[0.1, 0.2, 0.3] ) - msg = ("The indexed_timeseries field of IndexSeries is discouraged and will be deprecated in " - "a future version of NWB. Use the indexed_images field instead.") - with self.assertWarnsWith(PendingDeprecationWarning, msg): - iS = IndexSeries( - name='test_iS', + msg = ("The indexed_timeseries field of IndexSeries is deprecated. " + "Use the indexed_images field instead.") + kwargs = dict(name='test_iS', data=[1, 2, 3], unit='N/A', indexed_timeseries=ts, - timestamps=[0.1, 0.2, 0.3] - ) + timestamps=[0.1, 0.2, 0.3]) + + # create object with deprecated argument + with self.assertRaisesWith(ValueError, msg): + IndexSeries(**kwargs) + + # create object in construct mode, modeling the behavior of the ObjectMapper on read + # no warning or error should be raised + iS = IndexSeries.__new__(IndexSeries, in_construct_mode=True) + iS.__init__(**kwargs) + self.assertIs(iS.indexed_timeseries, ts) + def test_init_no_indexed_ts_or_timeseries(self): + msg = ("Either indexed_timeseries or indexed_images " + "must be provided when creating an IndexSeries.") + with self.assertRaisesWith(ValueError, msg): + IndexSeries(name='test_iS', data=[1, 2, 3], unit='N/A', timestamps=[0.1, 0.2, 0.3]) + class ImageMaskSeriesConstructor(TestCase): diff --git a/tests/unit/test_ophys.py b/tests/unit/test_ophys.py index 88bd24535..d484d9e01 100644 --- a/tests/unit/test_ophys.py +++ b/tests/unit/test_ophys.py @@ -131,11 +131,8 @@ def test_init(self): def test_manifold_deprecated(self): oc, device = self.set_up_dependencies() - msg = "The 'manifold' argument is deprecated in favor of 'origin_coords' and 'grid_spacing'." - with self.assertWarnsWith(DeprecationWarning, msg): - ImagingPlane( - name='test_imaging_plane', + kwargs = dict(name='test_imaging_plane', optical_channel=oc, description='description', device=device, @@ -143,16 +140,21 @@ def test_manifold_deprecated(self): imaging_rate=300., indicator='indicator', location='location', - manifold=(1, 1, (2, 2, 2)) - ) + manifold=(1, 1, (2, 2, 2))) + + # create object with deprecated argument + with self.assertRaisesWith(ValueError, msg): + ImagingPlane(**kwargs) + + # create object in construct mode, modeling the behavior of the ObjectMapper on read + # no warning or error should be raised + obj = ImagingPlane.__new__(ImagingPlane, in_construct_mode=True) + obj.__init__(**kwargs) def test_conversion_deprecated(self): oc, device = self.set_up_dependencies() - msg = "The 'conversion' argument is deprecated in favor of 'origin_coords' and 'grid_spacing'." - with self.assertWarnsWith(DeprecationWarning, msg): - ImagingPlane( - name='test_imaging_plane', + kwargs = dict(name='test_imaging_plane', optical_channel=oc, description='description', device=device, @@ -160,16 +162,21 @@ def test_conversion_deprecated(self): imaging_rate=300., indicator='indicator', location='location', - conversion=2.0 - ) + conversion=2.0) + + # create object with deprecated argument + with self.assertRaisesWith(ValueError, msg): + ImagingPlane(**kwargs) + + # create object in construct mode, modeling the behavior of the ObjectMapper on read + # no warning or error should be raised + obj = ImagingPlane.__new__(ImagingPlane, in_construct_mode=True) + obj.__init__(**kwargs) def test_unit_deprecated(self): oc, device = self.set_up_dependencies() - msg = "The 'unit' argument is deprecated in favor of 'origin_coords_unit' and 'grid_spacing_unit'." - with self.assertWarnsWith(DeprecationWarning, msg): - ImagingPlane( - name='test_imaging_plane', + kwargs = dict(name='test_imaging_plane', optical_channel=oc, description='description', device=device, @@ -178,8 +185,16 @@ def test_unit_deprecated(self): indicator='indicator', location='location', reference_frame='reference_frame', - unit='my_unit' - ) + unit='my_unit') + + # create object with deprecated argument + with self.assertRaisesWith(ValueError, msg): + ImagingPlane(**kwargs) + + # create object in construct mode, modeling the behavior of the ObjectMapper on read + # no warning or error should be raised + obj = ImagingPlane.__new__(ImagingPlane, in_construct_mode=True) + obj.__init__(**kwargs) class OnePhotonSeriesConstructor(TestCase): diff --git a/tests/unit/test_scratch.py b/tests/unit/test_scratch.py index 398ae2f78..7794fdee4 100644 --- a/tests/unit/test_scratch.py +++ b/tests/unit/test_scratch.py @@ -28,6 +28,42 @@ def test_constructor_list(self): self.assertListEqual(sd.data, [1, 2, 3, 4]) self.assertEqual(sd.description, 'test scratch') + def test_scratch_notes_deprecation(self): + msg = ("The `notes` argument of ScratchData.__init__ has been deprecated and will " + "be removed in PyNWB 4.0. Use description instead.") + with self.assertRaisesWith(ValueError, msg): + ScratchData(name='test', data=[1, 2, 3, 4, 5], notes='test notes') + + # create object in construct mode, modeling the behavior of the ObjectMapper on read + # should not raise error or warning + data = ScratchData.__new__(ScratchData, in_construct_mode=True) + data.__init__(name='test', data=[1, 2, 3, 4, 5], notes='test notes') + self.assertEqual(data.description, 'test notes') + + # test notes property + msg = ("Use of ScratchData.notes has been deprecated and will be removed in PyNWB 4.0. " + "Use ScratchData.description instead.") + + with self.assertWarnsWith(DeprecationWarning, msg): + data.notes + + # test notes setter + data = ScratchData(name='test', data=[1, 2, 3, 4, 5], description='test description') + with self.assertRaisesWith(ValueError, msg): + data.notes = 'test notes' + + def test_scratch_no_description(self): + with self.assertRaisesWith(ValueError, 'ScratchData.description is required.'): + ScratchData(name='test', data=[1, 2, 3, 4, 5]) + + def test_scratch_notes_and_description(self): + msg = ('Cannot provide both notes and description to ScratchData.__init__. The description ' + 'argument is recommended.') + data = ScratchData.__new__(ScratchData, in_construct_mode=True) + with self.assertRaisesWith(ValueError, msg): + data.__init__(name='test', data=[1, 2, 3, 4, 5], notes='test notes', + description='test description') + def test_add_scratch_int(self): ret = self.nwbfile.add_scratch(2, name='test', description='test data') self.assertIsInstance(ret, ScratchData) @@ -70,6 +106,14 @@ def test_add_scratch_dataframe_no_description(self): with self.assertRaisesWith(ValueError, msg): self.nwbfile.add_scratch(data, name='test') + def test_add_scratch_notes_and_description(self): + error_msg = 'Cannot call add_scratch with (notes or table_description) and description' + warning_msg = ("Use of the `notes` or `table_description` argument is deprecated and will be " + "removed in PyNWB 4.0. Use the `description` argument instead.") + with self.assertWarnsWith(DeprecationWarning, warning_msg): + with self.assertRaisesWith(ValueError, error_msg): + self.nwbfile.add_scratch([1, 2, 3, 4], name='test', description='test data', notes='test notes') + def test_add_scratch_container(self): data = TimeSeries(name='test_ts', data=[1, 2, 3, 4, 5], unit='unit', timestamps=[1.1, 1.2, 1.3, 1.4, 1.5]) self.nwbfile.add_scratch(data) @@ -106,6 +150,22 @@ def test_add_scratch_dynamictable(self): self.nwbfile.add_scratch(data) self.assertIs(self.nwbfile.get_scratch('test', convert=False), data) self.assertIs(self.nwbfile.scratch['test'], data) + + def test_add_scratch_notes_deprecation(self): + msg = ("Use of the `notes` or `table_description` argument is deprecated and will be removed in PyNWB 4.0. " + "Use the `description` argument instead.") + with self.assertWarnsWith(DeprecationWarning, msg): + self.nwbfile.add_scratch(name='test', data=[1, 2, 3, 4, 5], notes='test notes') + self.assertEqual(self.nwbfile.scratch['test'].description, 'test notes') + + def test_add_scratch_table_description_deprecation(self): + msg = ('Use of the `notes` or `table_description` argument is deprecated and will be removed in PyNWB 4.0. ' + 'Use the `description` argument instead.') + df = pd.DataFrame(data={'col1': [1, 2, 3, 4], 'col2': ['a', 'b', 'c', 'd']}) + with self.assertWarnsWith(DeprecationWarning, msg): + self.nwbfile.add_scratch(name='test', data=df, + table_description='test table_description') + self.assertEqual(self.nwbfile.scratch['test'].description, 'test table_description') def test_get_scratch_list_convert_false(self): self.nwbfile.add_scratch([1, 2, 3, 4], name='test', description='test description')