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

run conversion: Use context manager only in append mode #1180

Merged
merged 21 commits into from
Jan 24, 2025
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
11 changes: 6 additions & 5 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,21 +1,22 @@
# v0.7.0 (Upcoming)

## Deprecations
## Deprecations and Changes
* Interfaces and converters now have `verbose=False` by default [PR #1153](https://github.com/catalystneuro/neuroconv/pull/1153)
* Added `metadata` and `conversion_options` as arguments to `NWBConverter.temporally_align_data_interfaces` [PR #1162](https://github.com/catalystneuro/neuroconv/pull/1162)

## Bug Fixes
* `run_conversion` does not longer trigger append mode an index error when `nwbfile_path` points to a faulty file [PR #1180](https://github.com/catalystneuro/neuroconv/pull/1180)

## Features
* Added `metadata` and `conversion_options` as arguments to `NWBConverter.temporally_align_data_interfaces` [PR #1162](https://github.com/catalystneuro/neuroconv/pull/1162)
* Use the latest version of ndx-pose for `DeepLabCutInterface` and `LightningPoseDataInterface` [PR #1128](https://github.com/catalystneuro/neuroconv/pull/1128)

## Improvements
* Interfaces and converters now have `verbose=False` by default [PR #1153](https://github.com/catalystneuro/neuroconv/pull/1153)

* Simple writing no longer uses a context manager [PR #1180](https://github.com/catalystneuro/neuroconv/pull/1180)


# v0.6.7 (January 20, 2025)

## Deprecations
## Deprecations and Changes

## Bug Fixes
* Temporary set a ceiling for hdmf to avoid a chunking bug [PR #1175](https://github.com/catalystneuro/neuroconv/pull/1175)
Expand Down
64 changes: 43 additions & 21 deletions src/neuroconv/basedatainterface.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@
make_nwbfile_from_metadata,
make_or_load_nwbfile,
)
from .tools.nwb_helpers._metadata_and_file_helpers import _resolve_backend
from .tools.nwb_helpers._metadata_and_file_helpers import (
_resolve_backend,
configure_and_write_nwbfile,
)
from .utils import (
get_json_schema_from_method_signature,
load_dict_from_file,
Expand Down Expand Up @@ -163,7 +166,7 @@ def run_conversion(
Parameters
----------
nwbfile_path : FilePathType
Path for where the data will be written or appended.
Path for where to write or load (if overwrite=False) the NWBFile.
nwbfile : NWBFile, optional
An in-memory NWBFile object to write to the location.
metadata : dict, optional
Expand All @@ -182,32 +185,51 @@ def run_conversion(
Otherwise, all datasets will use default configuration settings.
"""

backend = _resolve_backend(backend, backend_configuration)
no_nwbfile_provided = nwbfile is None # Otherwise, variable reference may mutate later on inside the context
appending_to_in_memory_nwbfile = nwbfile is not None
file_initially_exists = Path(nwbfile_path).exists() if nwbfile_path is not None else False
appending_to_in_disk_nwbfile = file_initially_exists and not overwrite

if appending_to_in_disk_nwbfile and appending_to_in_memory_nwbfile:
raise ValueError(
"Cannot append to an existing file while also providing an in-memory NWBFile. "
"Either set overwrite=True to replace the existing file, or remove the nwbfile parameter to append to the existing file on disk."
)

if metadata is None:
metadata = self.get_metadata()
self.validate_metadata(metadata=metadata, append_mode=appending_to_in_disk_nwbfile)

if not appending_to_in_disk_nwbfile:
if appending_to_in_memory_nwbfile:
self.add_to_nwbfile(nwbfile=nwbfile, metadata=metadata, **conversion_options)
else:
nwbfile = self.create_nwbfile(metadata=metadata, **conversion_options)

configure_and_write_nwbfile(
nwbfile=nwbfile,
output_filepath=nwbfile_path,
backend=backend,
backend_configuration=backend_configuration,
)

else: # We are only using the context in append mode, see issue #1143

backend = _resolve_backend(backend, backend_configuration)
with make_or_load_nwbfile(
nwbfile_path=nwbfile_path,
nwbfile=nwbfile,
metadata=metadata,
overwrite=overwrite,
backend=backend,
verbose=getattr(self, "verbose", False),
) as nwbfile_out:

file_initially_exists = Path(nwbfile_path).exists() if nwbfile_path is not None else False
append_mode = file_initially_exists and not overwrite

self.validate_metadata(metadata=metadata, append_mode=append_mode)

with make_or_load_nwbfile(
nwbfile_path=nwbfile_path,
nwbfile=nwbfile,
metadata=metadata,
overwrite=overwrite,
backend=backend,
verbose=getattr(self, "verbose", False),
) as nwbfile_out:
if no_nwbfile_provided:
self.add_to_nwbfile(nwbfile=nwbfile_out, metadata=metadata, **conversion_options)

if backend_configuration is None:
backend_configuration = self.get_default_backend_configuration(nwbfile=nwbfile_out, backend=backend)
if backend_configuration is None:
backend_configuration = self.get_default_backend_configuration(nwbfile=nwbfile_out, backend=backend)

configure_backend(nwbfile=nwbfile_out, backend_configuration=backend_configuration)
configure_backend(nwbfile=nwbfile_out, backend_configuration=backend_configuration)

@staticmethod
def get_default_backend_configuration(
Expand Down
56 changes: 38 additions & 18 deletions src/neuroconv/nwbconverter.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
from .tools.nwb_helpers import (
HDF5BackendConfiguration,
ZarrBackendConfiguration,
configure_and_write_nwbfile,
configure_backend,
get_default_backend_configuration,
get_default_nwbfile_metadata,
Expand Down Expand Up @@ -243,35 +244,54 @@ def run_conversion(
" use Converter.add_to_nwbfile."
)

backend = _resolve_backend(backend, backend_configuration)
no_nwbfile_provided = nwbfile is None # Otherwise, variable reference may mutate later on inside the context

appending_to_in_memory_nwbfile = nwbfile is not None
file_initially_exists = Path(nwbfile_path).exists() if nwbfile_path is not None else False
append_mode = file_initially_exists and not overwrite
appending_to_in_disk_nwbfile = file_initially_exists and not overwrite

if appending_to_in_disk_nwbfile and appending_to_in_memory_nwbfile:
raise ValueError(
"Cannot append to an existing file while also providing an in-memory NWBFile. "
"Either set overwrite=True to replace the existing file, or remove the nwbfile parameter to append to the existing file on disk."
)

if metadata is None:
metadata = self.get_metadata()

self.validate_metadata(metadata=metadata, append_mode=append_mode)
self.validate_metadata(metadata=metadata, append_mode=appending_to_in_disk_nwbfile)
self.validate_conversion_options(conversion_options=conversion_options)

self.temporally_align_data_interfaces(metadata=metadata, conversion_options=conversion_options)

with make_or_load_nwbfile(
nwbfile_path=nwbfile_path,
nwbfile=nwbfile,
metadata=metadata,
overwrite=overwrite,
backend=backend,
verbose=getattr(self, "verbose", False),
) as nwbfile_out:
if no_nwbfile_provided:
if not appending_to_in_disk_nwbfile:

if appending_to_in_memory_nwbfile:
self.add_to_nwbfile(nwbfile=nwbfile, metadata=metadata, conversion_options=conversion_options)
else:
nwbfile = self.create_nwbfile(metadata=metadata, conversion_options=conversion_options)

configure_and_write_nwbfile(
nwbfile=nwbfile,
output_filepath=nwbfile_path,
backend=backend,
backend_configuration=backend_configuration,
)

else: # We are only using the context in append mode, see issue #1143

backend = _resolve_backend(backend, backend_configuration)
with make_or_load_nwbfile(
nwbfile_path=nwbfile_path,
nwbfile=nwbfile,
metadata=metadata,
overwrite=overwrite,
backend=backend,
verbose=getattr(self, "verbose", False),
) as nwbfile_out:
self.add_to_nwbfile(nwbfile=nwbfile_out, metadata=metadata, conversion_options=conversion_options)

if backend_configuration is None:
backend_configuration = self.get_default_backend_configuration(nwbfile=nwbfile_out, backend=backend)
if backend_configuration is None:
backend_configuration = self.get_default_backend_configuration(nwbfile=nwbfile_out, backend=backend)

configure_backend(nwbfile=nwbfile_out, backend_configuration=backend_configuration)
configure_backend(nwbfile=nwbfile_out, backend_configuration=backend_configuration)

def temporally_align_data_interfaces(
self, metadata: Optional[dict] = None, conversion_options: Optional[dict] = None
Expand Down
7 changes: 1 addition & 6 deletions src/neuroconv/tools/testing/data_interface_mixins.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,6 @@ def test_run_conversion_with_backend_configuration(self, setup_interface, tmp_pa
backend_configuration = self.interface.get_default_backend_configuration(nwbfile=nwbfile, backend=backend)
self.interface.run_conversion(
nwbfile_path=nwbfile_path,
nwbfile=nwbfile,
overwrite=True,
metadata=metadata,
backend_configuration=backend_configuration,
Expand Down Expand Up @@ -227,7 +226,6 @@ class TestNWBConverter(NWBConverter):
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,
Expand Down Expand Up @@ -889,7 +887,6 @@ def check_run_conversion_with_backend_configuration(
backend_configuration = self.interface.get_default_backend_configuration(nwbfile=nwbfile, backend=backend)
self.interface.run_conversion(
nwbfile_path=nwbfile_path,
nwbfile=nwbfile,
overwrite=True,
metadata=metadata,
backend_configuration=backend_configuration,
Expand Down Expand Up @@ -931,7 +928,6 @@ class TestNWBConverter(NWBConverter):
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,
Expand Down Expand Up @@ -1262,7 +1258,7 @@ def check_run_conversion_with_backend_configuration(
backend_configuration = self.interface.get_default_backend_configuration(nwbfile=nwbfile, backend=backend)
self.interface.run_conversion(
nwbfile_path=nwbfile_path,
nwbfile=nwbfile,
metadata=metadata,
overwrite=True,
backend_configuration=backend_configuration,
**self.conversion_options,
Expand Down Expand Up @@ -1303,7 +1299,6 @@ class TestNWBConverter(NWBConverter):
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,
Expand Down
19 changes: 19 additions & 0 deletions tests/test_ecephys/test_ecephys_interfaces.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from hdmf.testing import TestCase
from packaging.version import Version

from neuroconv import ConverterPipe
from neuroconv.datainterfaces import Spike2RecordingInterface
from neuroconv.tools.nwb_helpers import get_module
from neuroconv.tools.testing.mock_interfaces import (
Expand Down Expand Up @@ -158,6 +159,24 @@ def test_group_naming_not_adding_extra_devices(self, setup_interface):
assert len(nwbfile.devices) == 1
assert len(nwbfile.electrode_groups) == 4

def test_error_for_append_with_in_memory_file(self, setup_interface, tmp_path):

nwbfile_path = tmp_path / "test.nwb"
self.interface.run_conversion(nwbfile_path=nwbfile_path)

nwbfile = self.interface.create_nwbfile()

expected_error_message = (
"Cannot append to an existing file while also providing an in-memory NWBFile. "
"Either set overwrite=True to replace the existing file, or remove the nwbfile parameter to append to the existing file on disk."
)
with pytest.raises(ValueError, match=expected_error_message):
self.interface.run_conversion(nwbfile=nwbfile, nwbfile_path=nwbfile_path, overwrite=False)

converter = ConverterPipe(data_interfaces=[self.interface])
with pytest.raises(ValueError, match=expected_error_message):
converter.run_conversion(nwbfile=nwbfile, nwbfile_path=nwbfile_path, overwrite=False)


class TestAssertions(TestCase):
@pytest.mark.skipif(python_version.minor != 10, reason="Only testing with Python 3.10!")
Expand Down
Loading