BUNPC / pysnirf2

Python package for reading, writing and validating Shared Near Infrared Spectroscopy Format (SNIRF) files
GNU General Public License v3.0
15 stars 11 forks source link

ENH: Auto-close h5py.File using context manager #22

Closed larsoner closed 2 years ago

larsoner commented 2 years ago

Use context managers where easy to avoid a bunch of warnings at Python exit of the form:

ResourceWarning: unclosed file <_io.BufferedRandom name=57>
sstucker commented 2 years ago

This is great. On my todo list. does __del__ seem to reliably close the files on your end? I found it does not in IPython

larsoner commented 2 years ago

does del seem to reliably close the files on your end? I found it does not in IPython

I actually implemented a h5py_File_autoclose class that inherited from h5py.File that just had a __del__ method that called self.close at first. I think it worked at least some of the time (?), but settled on this since it's cleaner.

sstucker commented 2 years ago

Would it be more robust, maybe at expense of speed, to just open and close the file every time it is read and written?

larsoner commented 2 years ago

Could be. What's done now with the context manager seems clean enough, though. With the current implementation it's clear that the method opens one file, writes to it a bit, then closes it. And no need to worry about opening and closing repeatedly.

sstucker commented 2 years ago

The paradigm for the whole library, though, is that creating a Snirf instance opens a file in "read-write" mode, and calling close() on it or letting it exit scope should close the underlying h5py.File interface and calling save() without new arguments writes to the open file.

Calling save(<with a new path>) does as you describe

opens one file, writes to it a bit, then closes it.

So the user is to do their own teardown snirf.close() or use

with Snirf(<path>) as snirf:
      # ...

Probably this should be better documented. The method snirfLoad is a bit of a misnomer because the Snirf instance it returns is actually a wrapper around an open file, not just loaded data

sstucker commented 2 years ago

I actually implemented a h5py_File_autoclose class that inherited from h5py.File that just had a del method that called self.close at first. I think it worked at least some of the time (?), but settled on this since it's cleaner.

Apparently __del__ is not called when all references are gone or when an object is deled but at the nondeterministic whims of the gc. So that explains the mixed results I was getting in IPython