From 2f2a494bbdbb8f6f44c987bd9feb27e0e8ff9984 Mon Sep 17 00:00:00 2001 From: Ryan Ly Date: Fri, 8 Nov 2024 13:14:52 -0500 Subject: [PATCH 1/2] Make SpatialSeries reference_frame optional (#1986) --- CHANGELOG.md | 1 + src/pynwb/behavior.py | 4 ++-- tests/integration/hdf5/test_behavior.py | 11 +++++++++++ tests/unit/test_behavior.py | 9 +++++++++ 4 files changed, 23 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 16e504659..9f627141a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ ### Bug fixes - Fixed bug in how `ElectrodeGroup.__init__` validates its `position` argument. @oruebel [#1770](https://github.com/NeurodataWithoutBorders/pynwb/pull/1770) +- Changed `SpatialSeries.reference_frame` from required to optional as specified in the schema. @rly [#1986](https://github.com/NeurodataWithoutBorders/pynwb/pull/1986) ## PyNWB 2.8.2 (September 9, 2024) diff --git a/src/pynwb/behavior.py b/src/pynwb/behavior.py index 1b3078535..286ec43bd 100644 --- a/src/pynwb/behavior.py +++ b/src/pynwb/behavior.py @@ -28,8 +28,8 @@ class SpatialSeries(TimeSeries): 'or 3 columns, which represent x, y, and z.')}, {'name': 'bounds', 'type': list, 'shape': ((1, 2), (2, 2), (3, 2)), 'default': None, 'doc': 'The boundary range (min, max) for each dimension of data.'}, - {'name': 'reference_frame', 'type': str, # required - 'doc': 'description defining what the zero-position is'}, + {'name': 'reference_frame', 'type': str, + 'doc': 'description defining what the zero-position is', 'default': None}, {'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', diff --git a/tests/integration/hdf5/test_behavior.py b/tests/integration/hdf5/test_behavior.py index d4b230bcc..eb5974449 100644 --- a/tests/integration/hdf5/test_behavior.py +++ b/tests/integration/hdf5/test_behavior.py @@ -15,3 +15,14 @@ def setUpContainer(self): reference_frame='reference_frame', timestamps=[1., 2., 3.] ) + + +class TestSpatialSeriesMinIO(AcquisitionH5IOMixin, TestCase): + + def setUpContainer(self): + """ Return the test TimeSeries to read/write """ + return SpatialSeries( + name='test_sS', + data=np.ones((3, 2)), + timestamps=[1., 2., 3.] + ) diff --git a/tests/unit/test_behavior.py b/tests/unit/test_behavior.py index 6bcf1a9eb..be43eb8b3 100644 --- a/tests/unit/test_behavior.py +++ b/tests/unit/test_behavior.py @@ -21,6 +21,15 @@ def test_init(self): self.assertEqual(sS.bounds, [(-1,1),(-1,1),(-1,1)]) self.assertEqual(sS.reference_frame, 'reference_frame') + def test_init_minimum(self): + sS = SpatialSeries( + name='test_sS', + data=np.ones((3, 2)), + timestamps=[1., 2., 3.] + ) + assert sS.bounds is None + assert sS.reference_frame is None + def test_set_unit(self): sS = SpatialSeries( name='test_sS', From e55de6cbebf9c5768510ce07cad2b6abe17dfb7d Mon Sep 17 00:00:00 2001 From: Steph Prince <40640337+stephprince@users.noreply.github.com> Date: Fri, 8 Nov 2024 11:25:08 -0800 Subject: [PATCH 2/2] Warn if writing a file without the appropriate extension (#1978) * add warning for non nwb extension * add tests and update test filenames * update CHANGELOG.md * Remove comment in tests --- CHANGELOG.md | 3 +++ src/pynwb/__init__.py | 4 ++++ tests/integration/hdf5/test_file_copy.py | 4 ++-- tests/integration/hdf5/test_io.py | 21 +++++++++++++++++++-- tests/unit/test_icephys_metadata_tables.py | 4 ++-- 5 files changed, 30 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f627141a..25317ee8f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,9 @@ - Fixed bug in how `ElectrodeGroup.__init__` validates its `position` argument. @oruebel [#1770](https://github.com/NeurodataWithoutBorders/pynwb/pull/1770) - Changed `SpatialSeries.reference_frame` from required to optional as specified in the schema. @rly [#1986](https://github.com/NeurodataWithoutBorders/pynwb/pull/1986) +### Enhancements and minor changes` +- Added warning when writing files with `NWBHDF5IO` without the `.nwb` extension. @stephprince [#1978](https://github.com/NeurodataWithoutBorders/pynwb/pull/1978) + ## PyNWB 2.8.2 (September 9, 2024) ### Enhancements and minor changes diff --git a/src/pynwb/__init__.py b/src/pynwb/__init__.py index 1d109abe3..2ac87a3ea 100644 --- a/src/pynwb/__init__.py +++ b/src/pynwb/__init__.py @@ -397,6 +397,10 @@ def __init__(self, **kwargs): if mode in io_modes_that_create_file or manager is not None or extensions is not None: load_namespaces = False + if mode in io_modes_that_create_file and not str(path).endswith('.nwb'): + warn(f"The file path provided: {path} does not end in '.nwb'. " + "It is recommended that NWB files using the HDF5 backend use the '.nwb' extension.", UserWarning) + if load_namespaces: tm = get_type_map() super().load_namespaces(tm, path, file=file_obj, driver=driver, aws_region=aws_region) diff --git a/tests/integration/hdf5/test_file_copy.py b/tests/integration/hdf5/test_file_copy.py index b491586fb..be23f74ca 100644 --- a/tests/integration/hdf5/test_file_copy.py +++ b/tests/integration/hdf5/test_file_copy.py @@ -9,8 +9,8 @@ class TestFileCopy(TestCase): def setUp(self): - self.path1 = "test_a.h5" - self.path2 = "test_b.h5" + self.path1 = "test_a.nwb" + self.path2 = "test_b.nwb" def tearDown(self): if os.path.exists(self.path1): diff --git a/tests/integration/hdf5/test_io.py b/tests/integration/hdf5/test_io.py index e1ce4269b..a97eb1b63 100644 --- a/tests/integration/hdf5/test_io.py +++ b/tests/integration/hdf5/test_io.py @@ -313,7 +313,7 @@ def setUp(self): self.nwbfile = NWBFile(session_description='a', identifier='b', session_start_time=datetime(1970, 1, 1, 12, tzinfo=tzutc())) - self.path = "test_pynwb_io_hdf5_h5dataIO.h5" + self.path = "test_pynwb_io_hdf5_h5dataIO.nwb" def tearDown(self): remove_test_file(self.path) @@ -428,7 +428,7 @@ def setUp(self): self.nwbfile = NWBFile(session_description='a test NWB File', identifier='TEST123', session_start_time=datetime(1970, 1, 1, 12, tzinfo=tzutc())) - self.path = "test_pynwb_io_nwbhdf5.h5" + self.path = "test_pynwb_io_nwbhdf5.nwb" def tearDown(self): remove_test_file(self.path) @@ -532,6 +532,23 @@ def test_round_trip_with_pathlib_path(self): read_file = io.read() self.assertContainerEqual(read_file, self.nwbfile) + def test_warn_for_nwb_extension(self): + """Creating a file with an extension other than .nwb should raise a warning""" + pathlib_path = Path(self.path).with_suffix('.h5') + + with self.assertWarns(UserWarning): + with NWBHDF5IO(pathlib_path, 'w') as io: + io.write(self.nwbfile) + with self.assertWarns(UserWarning): + with NWBHDF5IO(str(pathlib_path), 'w') as io: + io.write(self.nwbfile) + + # should not warn on read or append + with NWBHDF5IO(str(pathlib_path), 'r') as io: + io.read() + with NWBHDF5IO(str(pathlib_path), 'a') as io: + io.read() + def test_can_read_current_nwb_file(self): with NWBHDF5IO(self.path, 'w') as io: io.write(self.nwbfile) diff --git a/tests/unit/test_icephys_metadata_tables.py b/tests/unit/test_icephys_metadata_tables.py index b31ee9215..a357f3288 100644 --- a/tests/unit/test_icephys_metadata_tables.py +++ b/tests/unit/test_icephys_metadata_tables.py @@ -82,7 +82,7 @@ def setUp(self): sweep_number=np.uint64(15) ) self.nwbfile.add_acquisition(self.response) - self.path = 'test_icephys_meta_intracellularrecording.h5' + self.path = 'test_icephys_meta_intracellularrecording.nwb' def tearDown(self): remove_test_file(self.path) @@ -1037,7 +1037,7 @@ class NWBFileTests(TestCase): """ def setUp(self): warnings.simplefilter("always") # Trigger all warnings - self.path = 'test_icephys_meta_intracellularrecording.h5' + self.path = 'test_icephys_meta_intracellularrecording.nwb' def tearDown(self): remove_test_file(self.path)