respec / HSPsquared

Hydrologic Simulation Program Python (HSPsquared)
GNU Affero General Public License v3.0
43 stars 17 forks source link

HDF5 Class can leave open connections in IPython #83

Closed ptomasula closed 2 years ago

ptomasula commented 2 years ago

During our workshop we noticed that instances of the HDF5 class can leave connections open to the HDF5, file that persist even when the del method is called on the instance. This behavior was only observed in an IPython environment (e.g. jupyter lab). With some additional testing, I determined that this occurs when the instance of that object was called as the last line in the cell. This typically does some display feature in IPython, but for this particular class is also appear to cause the instance to somehow become referenced by the cell. The results is that the open _store attribute is keeping the connection to HDF5 even when the instance has been deleted.

image
ptomasula commented 2 years ago

@aufdenkampe, do you have a better sense of what actually occurs when I reference an instance of an object as the last line in a jupyter lab cell? If might yet be possible to resolve this will some appropriate Dunder methods for that class.

In the meantime, I'm going to implement the __enter__ and __exit__ dunder methods so that we can utilize that class with context managers. That will prevent us from accidently creating connection that we cannot yet figure out how to close without using the pytables library the table.file._open_files.close_all() function or closing the kernel.

aufdenkampe commented 2 years ago

@ptomasula, that sounds like a great plan for now.

I've been reading up a bit on context managers and Python with statements here: https://realpython.com/python-with-statement/

I like your idea of doing something like this:

with HSP2IO.hdf.HDF5(output_hdf5_path) as hdf5_instance:
    io_manager = HSP2IO.io.IOManager(hdf5_instance)
    HSP2.main(io_manager, saveall=True)
ptomasula commented 2 years ago

Thanks @aufdenkampe. I just implemented support for that with commit f080a83.

aufdenkampe commented 2 years ago

@ptomasula, thanks for that! To merge with my notebooks, I created:

Thanks @PaulDudaRESPEC for that review and merge!

@ptomasula, before we issue this release, I'll update the tutorial notebooks to use that with as approach.

aufdenkampe commented 2 years ago

Tutorials updated with 5ca8619