run_conversion fails to add data when in-memory nwbfile is passed. Feature or bug?

[h-mayorquin]

Summary: when using converter.run_conversion and the nwbfile argument is used to pass an in-memory nwbfile then the data from the converter is not added. This sounds like a bug to me but I don't know if this was intended behavior at some point.

What do you think? @pauladkisson @weiglszonja @bendichter @alessandratrapani

The reason for this behavior is found on these lines that only call add_to_nwbfile if no nwbfile is provided.

neuroconv/src/neuroconv/nwbconverter.py

Lines 272 to 274 in 05423d1

	if no_nwbfile_provided:
	self.add_to_nwbfile(nwbfile=nwbfile_out, metadata=metadata, conversion_options=conversion_options)

For a concrete example, see the following:

from neuroconv.tools.testing.mock_interfaces import MockRecordingInterface, MockSortingInterface, MockImagingInterface
from neuroconv import ConverterPipe
from tempfile import TemporaryDirectory



imaging_interface = MockImagingInterface()
recording_interface = MockRecordingInterface()
sorting_interface = MockSortingInterface()

nwbfile = imaging_interface.create_nwbfile()

converter = ConverterPipe(data_interfaces=[recording_interface, sorting_interface])

with TemporaryDirectory() as tmpdir:
    tmp_path = Path(tmpdir)
    nwbfile_path = tmp_path / "test_conversion.nwb"
    converter.run_conversion(nwbfile_path=nwbfile_path, nwbfile=nwbfile)
    
    from pynwb import NWBHDF5IO 
    with NWBHDF5IO(str(nwbfile_path), 'r') as io:
        nwbfile = io.read()
        assert nwbfile.units is not None
        assert len(nwbfile.aquisition) > 1
        

Both assertions fail no matter the order. In other words, neither the recording interface nor sorting interface data are added when calling run conversion.

Relevant: the existence of this behavior is used in our testing here:

neuroconv/src/neuroconv/tools/testing/data_interface_mixins.py

Lines 227 to 238 in 05423d1

	nwbfile = converter.create_nwbfile(metadata=metadata, conversion_options=conversion_options)
	backend_configuration = converter.get_default_backend_configuration(nwbfile=nwbfile, backend=backend)
	converter.run_conversion(
	nwbfile_path=nwbfile_path,
	nwbfile=nwbfile,
	overwrite=True,
	metadata=metadata,
	backend_configuration=backend_configuration,
	conversion_options=conversion_options,
	)

Where the test would fail if null behavior were to change as the converter would try to add the data again.

[pauladkisson]

Seems like a bug to me.