[Backend Configuration IVa] Add backend control to context tool

[CodyCBakerPhD]

Breaking up the integration into the core classes into smaller pieces

This is the first step - adding backend keyword and functionality into the context tool used to write the majority of NWB files through NeuroConv

[h-mayorquin]

I suggest a possible and small improvement to the error function but this looks OK.

Side comment:
Every time that I look at the make_or_load_nwbfile I feel that it could be simplified. Even on the name is doing too many things and the control flow at the end looks very complicated. Maybe there is no better way here if we want a context manager.

I don't know if auto merge is set. I will first figure that one out.

[CodyCBakerPhD]

Maybe there is no better way here if we want a context manager.

Yeah, we wanted a context manager, and we wanted to reduce duplicated code since all the complexity of that function was previously sharded across the entire library

The most I could reduce it now is perhaps to split the 'make' and 'load' parts to separate contexts

[codecov]

Codecov Report

Attention: Patch coverage is 92.30769% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 91.37%. Comparing base (def8e57) to head (9c550eb).
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #800      +/-   ##
==========================================
- Coverage   91.38%   91.37%   -0.01%     
==========================================
  Files         121      121              
  Lines        6300     6308       +8     
==========================================
+ Hits         5757     5764       +7     
- Misses        543      544       +1     

Flag	Coverage Δ
unittests	91.37% <92.30%> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
...nv/tools/nwb_helpers/_metadata_and_file_helpers.py	95.74% <92.30%> (-0.77%)	⬇️

[h-mayorquin]

Thanks for the work on this.

The most I could reduce it now is perhaps to split the 'make' and 'load' parts to separate contexts

Yes, I think the make part does not really require a context manager, right? I feel the main responsability of this -and its most common use- is to write. But I think that mos of the complexity comes from the loading to append part.

Relevant to the discussion in #801

[CodyCBakerPhD]

Yes, I think the make part does not really require a context manager, right?

The context is to trigger write after making or loading; so they would still be make_and_write_nwbfile and append_and_write_nwbfile with duplicated switching logic everywhere

[h-mayorquin]

The context is to trigger write after making or loading; so they would still be make_and_write_nwbfile and append_and_write_nwbfile with duplicated switching logic everywhere

or passing a in-memory nwbfile, right? the commontality is still writing I feel.

I just opened the function to confirm that we have the nwbfile argument and then I could not figure out in 3 minutes how it is used.

I copied the code to illustrate this, I thought about it another half a minute and I can't figure out which branch do I go when I passed an nwbfile. I think is the first yield?

Oh, no, the path is optional, which means that it does not neceesarily write.

OK, forget about it. I am confused.

    load_kwargs = dict()
    success = True
    file_initially_exists = nwbfile_path_in.is_file() if nwbfile_path_in is not None else None
    if nwbfile_path_in:
        load_kwargs.update(path=nwbfile_path_in)
        if file_initially_exists and not overwrite:
            load_kwargs.update(mode="r+", load_namespaces=True)
        else:
            load_kwargs.update(mode="w")
        io = NWBHDF5IO(**load_kwargs)
    try:
        if load_kwargs.get("mode", "") == "r+":
            nwbfile = io.read()
        elif nwbfile is None:
            nwbfile = make_nwbfile_from_metadata(metadata=metadata)
        yield nwbfile
    except Exception as e:
        success = False
        raise e
    finally:
        if nwbfile_path_in:
            try:
                if success:
                    io.write(nwbfile)

                    if verbose:
                        print(f"NWB file saved at {nwbfile_path_in}!")
            finally:
                io.close()

                if not success and not file_initially_exists:
                    nwbfile_path_in.unlink()

[CodyCBakerPhD]

or passing a in-memory nwbfile, right? the commontality is still writing I feel.

Yes, perhaps a third function write_nwbfile just for that one case too

[h-mayorquin]

Creation of an nwbfile does not require a context manager unless it is opened for append mode, right? Writing on the other hand does always benefit for it so the io is closed.

Maybe the open and append is one of the valid use case for nested context managers:

https://discuss.codecademy.com/t/faq-context-managers-nested-context-managers/601741/2

But that could end up looking worse. No idea.

[CodyCBakerPhD]

Creation of an nwbfile does not require a context manager unless it is opened for append mode, right? Writing on the other hand does always benefit for it so the io is closed.

Hence why I'd have make_and_write_nwbfile coupled together as well as write_nwbfile available separately

A double context could be used in append_and_write_nwbfile, but I don't think it would be outright necessary internally

[h-mayorquin]

Hence why I'd have make_and_write_nwbfile coupled together as well as write_nwbfile available separately

Makes sense to me. But would it make more sense for the first to be load_and_write?

[CodyCBakerPhD]

load_and_write

Sure, though that could also be interpreted as 'read from source and write to destination' (mode="r") instead of 'read from source and rewrite to same source' (mode="a"/"r+", explicit name being 'append')

[h-mayorquin]

To be honest, I kind of though that we could do the first but that does not work because we only have one path argument anyway. load_and_append or something else could be more precise or something better.