catalystneuro / ndx-photometry

photometry extension for nwb
BSD 3-Clause "New" or "Revised" License
0 stars 1 forks source link

Incompatibility with HDMF>=3.7.0 #8

Closed pauladkisson closed 6 months ago

pauladkisson commented 1 year ago

When running my conversion w/ the latest HDMF versions (>=3.7.0), I get the following error during the construction of the FiberPhotometry object:

Traceback (most recent call last):
  File "/Users/pauladkisson/Documents/CatalystNeuro/NWB/DattaConv/catalystneuro/datta-lab-to-nwb/src/datta_lab_to_nwb/markowitz_gillis_nature_2023/convert_session.py", line 117, in <module>
    session_to_nwb(
  File "/Users/pauladkisson/Documents/CatalystNeuro/NWB/DattaConv/catalystneuro/datta-lab-to-nwb/src/datta_lab_to_nwb/markowitz_gillis_nature_2023/convert_session.py", line 87, in session_to_nwb
    converter.run_conversion(metadata=metadata, nwbfile_path=nwbfile_path, conversion_options=conversion_options)
  File "/Users/pauladkisson/Documents/CatalystNeuro/NWB/neuroconv/src/neuroconv/nwbconverter.py", line 160, in run_conversion
    self.add_to_nwbfile(nwbfile_out, metadata, conversion_options)
  File "/Users/pauladkisson/Documents/CatalystNeuro/NWB/neuroconv/src/neuroconv/nwbconverter.py", line 114, in add_to_nwbfile
    data_interface.add_to_nwbfile(
  File "/Users/pauladkisson/Documents/CatalystNeuro/NWB/DattaConv/catalystneuro/datta-lab-to-nwb/src/datta_lab_to_nwb/markowitz_gillis_nature_2023/fiberphotometryinterface.py", line 143, in add_to_nwbfile
    FiberPhotometry(
  File "/opt/anaconda3/envs/datta_env_debug/lib/python3.10/site-packages/hdmf/utils.py", line 644, in func_call
    return func(args[0], **pargs)
  File "/opt/anaconda3/envs/datta_env_debug/lib/python3.10/site-packages/hdmf/build/classgenerator.py", line 320, in __init__
    setattr(self, f, arg_val)
  File "/opt/anaconda3/envs/datta_env_debug/lib/python3.10/site-packages/hdmf/container.py", line 494, in container_setter
    v.parent = self
  File "/opt/anaconda3/envs/datta_env_debug/lib/python3.10/site-packages/hdmf/container.py", line 418, in parent
    if child.table.parent is None:
AttributeError: 'NoneType' object has no attribute 'parent'
weiglszonja commented 1 year ago

@rly do you have any suggestions how to fix this?

rly commented 1 year ago

Hi, this is due to a combination of two issues. One is that HDMF 3.7.0 introduced a check to verify that the referenced table of a DynamicTableRegion has a shared ancestor with the object that the DTR is being added to, to prevent the user from creating invalid files where the referenced table is not in the file. That check assumed that the referenced table is set. But that is not always the case, as we see in this error. I'll create an issue in HDMF to not assume this.

In the meantime, the root trickiness that causes this error is that the FibersTable contains DynamicTableRegion columns that reference other tables, but the FibersTable is not initialized with those other tables. So when you call FibersTable.add_row, you have to use this hacky code https://github.com/catalystneuro/ndx-photometry/blob/main/src/pynwb/ndx_photometry/fibers_table.py#L46-L53 to check for the existence of the other tables in the parent NWB file, find the tables, and set them accordingly in order to add a row with a reference to the other tables. This makes strong assumptions that 1) the referenced tables have been added to the NWB file, and 2) that they live in nwbfile.lab_meta_data['fiber_photometry'].

An alternative approach is to initialize the FibersTable using the following:

fibers_table = FibersTable(
    description="fibers table",
    target_tables={
        'excitation_source': excitationsources_table,
        'photodetector': photodetectors_table,
        'fluorophores': fluorophores_table,
    },
)

Then you can run

fibers_table.add_fiber(
    excitation_source=0, #integers indicated rows of excitation sources table
    photodetector=0,
    fluorophores=[0], #potentially multiple fluorophores, so list of indices
    location='my location',
    notes='notes'
)

before creating a FiberPhotometry object and putting it in the NWBFile. I think this is more flexible and elegant, at the cost of this special setup.

So while we sort out this issue in HDMF, I recommend:

  1. change the readme example to initialize FibersTable as above and remove the note "# Important: we add the fibers to the fibers table after adding the metadata. This ensures that we can find this data in their tables of origin"
  2. create a custom FibersTable.__init__ that checks that the target tables are set on initialization, since these DTRs are required columns
  3. remove the custom add_fiber function (or keep it just in case)

Since I have thought about this, I'm happy to submit a PR with this. Let me know what you think.

weiglszonja commented 11 months ago

@rly Thank you that would be awesome!