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

Creating external links via TimeSeries(data=timeseries,...) broken #1767

Closed
oruebel opened this issue Aug 24, 2023 · 0 comments
Closed

Creating external links via TimeSeries(data=timeseries,...) broken #1767

oruebel opened this issue Aug 24, 2023 · 0 comments
Labels
category: bug errors in the code or code behavior priority: low alternative solution already working and/or relevant to only specific user(s)

Comments

@oruebel
Copy link
Contributor

oruebel commented Aug 24, 2023

Creating external links with the logic of setting TimeSeries(data=timeseries,...) and TimeSeries(timestamps=timeseries, ...) appears to be broken. Creating an external link with HDF5IO works:

timestamps=H5DataIO(
data=data_file_obt.get_acquisition('data_ts').timestamps,
link_data=True # test with setting link data

But using the approach with TimeSeries(data=timeseries, ...)

data=data_file_obt.get_acquisition('data_ts'), # test direct link

results in a broken external link:

  ...hdmf/backends/hdf5/h5tools.py:641: BrokenLinkWarning: Path to Group altered/broken at /acquisition/test_mod_ts/timestamps
    warnings.warn('Path to Group altered/broken at ' + os.path.join(h5obj.name, k), BrokenLinkWarning)

Originally posted by @oruebel in #1766 (comment)

How to test

#1766 adds TestTimeSeriesModularLinkViaTimeSeries to test this issue. This test is currently being skipped. Re-enabling the test will trigger the issue.

Further details

Looking at the generated file with the links, it appears that the path to the external file being set correctly but the path to the object is not being set correctly when using the TimeSeries(data=timeseries,...) approach:

data                     External Link {test_time_series_modular_data.nwb//data}
timestamps               External Link {test_time_series_modular_data.nwb//acquisition/data_ts/timestamps}

I.e. it should be External Link {test_time_series_modular_data.nwb//acquisition/data_ts/data} instead. The issue appears to be in how the path within the file is being determined. I'm not sure yet what the fix for this issue is, but I think it is likely either part of TimeSeriesMap.data_attr and TimeSeriesMap.timestamps_attr or a bug in HDF5IO.write_link

Workaround

As a workaround we can use H5DataIO to explicitly link to the external file:

TimeSeries(...
    timestamps=H5DataIO( 
         data=data_file_obt.get_acquisition('data_ts').timestamps, 
         link_data=True  # test with setting link data)
    ...)

or alternatively, when using NWBHDF5IO.write(..., link_data=True) (which is the default), we can omit wrapping with H5DataIO and simply set the timestamps to the h5py.Dataset:

TimeSeries(...
    timestamps=data_file_obt.get_acquisition('data_ts').timestamps, 
    ...)
@oruebel oruebel added category: bug errors in the code or code behavior priority: low alternative solution already working and/or relevant to only specific user(s) labels Aug 24, 2023
@oruebel oruebel closed this as completed in 047c37d Feb 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: bug errors in the code or code behavior priority: low alternative solution already working and/or relevant to only specific user(s)
Projects
None yet
Development

No branches or pull requests

1 participant