Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix reading file with linked TimeSeriesReferenceVectorData #1865

Merged
merged 4 commits into from
Mar 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
# PyNWB Changelog

## PyNWB 2.6.1 (Upcoming)
## PyNWB 2.6.1 (March 25, 2024)

### Bug fixes
- Fix bug with reading file with linked `TimeSeriesReferenceVectorData` @rly [#1865](https://github.com/NeurodataWithoutBorders/pynwb/pull/1865)
- Fix bug where extra keyword arguments could not be passed to `NWBFile.add_{x}_column`` for use in custom `VectorData`` classes. @rly [#1861](https://github.com/NeurodataWithoutBorders/pynwb/pull/1861)

## PyNWB 2.6.0 (February 21, 2024)
Expand Down
8 changes: 7 additions & 1 deletion src/pynwb/io/epoch.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from hdmf.build import LinkBuilder
from hdmf.common.table import VectorData
from hdmf.common.io.table import DynamicTableMap

Expand All @@ -8,13 +9,18 @@

@register_map(TimeIntervals)
class TimeIntervalsMap(DynamicTableMap):
pass

@DynamicTableMap.constructor_arg('columns')
def columns_carg(self, builder, manager):
# handle the case when a TimeIntervals is read with a non-TimeSeriesReferenceVectorData "timeseries" column
# this is the case for NWB schema v2.4 and earlier, where the timeseries column was a regular VectorData.
timeseries_builder = builder.get('timeseries')

# handle the case when the TimeIntervals has a "timeseries" column that is a link (e.g., it exists in
# a different file that was shallow copied to this file).
if isinstance(timeseries_builder, LinkBuilder):
timeseries_builder = timeseries_builder.builder

Check warning on line 22 in src/pynwb/io/epoch.py

View check run for this annotation

Codecov / codecov/patch

src/pynwb/io/epoch.py#L22

Added line #L22 was not covered by tests

# if we have a timeseries column and the type is VectorData instead of TimeSeriesReferenceVectorData
if (timeseries_builder is not None and
timeseries_builder.attributes['neurodata_type'] != 'TimeSeriesReferenceVectorData'):
Expand Down
44 changes: 44 additions & 0 deletions tests/integration/hdf5/test_file_copy.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import os
from pynwb.base import TimeSeriesReference
from pynwb import NWBHDF5IO
from pynwb.testing import TestCase
from pynwb.testing.mock.file import mock_NWBFile
from pynwb.testing.mock.base import mock_TimeSeries


class TestFileCopy(TestCase):

def setUp(self):
self.path1 = "test_a.h5"
self.path2 = "test_b.h5"

def tearDown(self):
if os.path.exists(self.path1):
os.remove(self.path1)
if os.path.exists(self.path2):
os.remove(self.path2)

def test_copy_file_link_timeintervals_timeseries(self):
"""Test copying a file with a TimeSeriesReference in a TimeIntervals object and reading that copy.

Based on https://github.com/NeurodataWithoutBorders/pynwb/issues/1863
"""
new_nwb = mock_NWBFile()
test_ts = mock_TimeSeries(name="test_ts", timestamps=[1.0, 2.0, 3.0], data=[1.0, 2.0, 3.0])
new_nwb.add_acquisition(test_ts)
new_nwb.add_trial(start_time=1.0, stop_time=2.0, timeseries=[test_ts])

with NWBHDF5IO(self.path1, 'w') as io:
io.write(new_nwb)

with NWBHDF5IO(self.path1, 'r') as base_io:
# the TimeIntervals object is copied but the TimeSeriesReferenceVectorData is linked
nwb_add = base_io.read().copy()
with NWBHDF5IO(self.path2, 'w', manager=base_io.manager) as out_io:
out_io.write(nwb_add)

with NWBHDF5IO(self.path2, 'r') as io:
nwb = io.read()
ts_val = nwb.trials["timeseries"][0][0]
assert isinstance(ts_val, TimeSeriesReference)
assert ts_val.timeseries is nwb.acquisition["test_ts"]
Loading