hdmf-dev / hdmf-zarr

Zarr I/O backend for HDMF
https://hdmf-zarr.readthedocs.io/
Other
7 stars 6 forks source link

[Bug]: Reading and exporting Zarr NWB fails to copy over table information #208

Open rcpeene opened 3 months ago

rcpeene commented 3 months ago

What happened?

I am trying to modify an existing zarr nwb file in memory and export a copy with those modifications. Specifically, I am trying to append columns to the nwb.electrodes table. More details discussed here: https://github.com/NeurodataWithoutBorders/helpdesk/discussions/86

After running code similar to the snippet below, the resulting exported NWB does not contain any of the original columns from nwb.electrodes except 'group'. It also includes the columns I have added (x, y, and z). Printing the electrodes table in memory after reading, before exporting, does show all columns and their names.

I examined the nwb.intervals tables as well and they also are missing all columns in the exported nwb.

Steps to Reproduce

read_io = NWBZarrIO('my_file.nwb', mode='a')
    nwb = read_io.read()
    # placeholder information
    xs, ys, zs = [np.rand(len(nwb.electrodes)) for i in range(3)]

    nwb.electrodes.add_column('x', 'ccf x coordinate', data=xs)
    nwb.electrodes.add_column('y', 'ccf y coordinate', data=ys)
    nwb.electrodes.add_column('z', 'ccf z coordinate', data=zs)

    nwbfile_output_path = results_folder / f"{nwbfile_input_path.stem}.nwb"
    with io_class(str(nwbfile_output_path), "w") as export_io:
        export_io.export(src_io=read_io, nwbfile=nwb)

Traceback

There is no traceback. The code exports without runtime errors.

Operating System

Linux

Python Executable

Conda

Python Version

3.9

Package Versions

hdmf-zarr==0.8.0

Code of Conduct

alejoe91 commented 3 months ago

@rcpeene see this issue: https://github.com/hdmf-dev/hdmf-zarr/issues/179

rcpeene commented 3 months ago

The solution mentioned there seems to work. Namely, passing write_args={'link_data': False} to the export call. It is not clear to me if this is intended behavior or merely a temporary solution. The electrodes table and intervals table are not information that is supposed to belong in a separate file. In other words, I think they should be copied over with the default export behavior.

mavaylon1 commented 3 months ago

@rcpeene We are working on a fix #194. Ideally rolled out by the end of the week.