Pandas 3.0, create new HDF5 close() method#216
Pandas 3.0, create new HDF5 close() method#216rburghol wants to merge 11 commits intorespec:developfrom
Conversation
There was a problem hiding this comment.
!! I'm not sure how this fits in with the pandas 3.0 patch, but we should have a look at this as a team, for sure. Both implementations break an important (and oft un-enforced in non-aerospace grade code) rule to never branch on bare float equality tests, instead it's safer to set a tolerance and check against it via abs(a - b) > tol or whatever. This would be a huge undertaking for us to track down comprehensively for our code base, but maybe we do it whenever we find cases like this where our code branches differently from the fortran and then our tests fail. This is another great reason to promote the checks in the cmd_regression into the test suite, if they're not there already.
There was a problem hiding this comment.
This edit is not needed since the issue is on the caller side not using this class as a context manager. That said, an explicit close method is a fine thing to add, but we should be guiding folks to use the with statement rather than manually closing things.
| hdf5_instance = HDF5(h5file) | ||
| io_manager = IOManager(hdf5_instance) | ||
| main(io_manager, saveall=saveall, jupyterlab=compress) | ||
| hdf5_instance.close() |
There was a problem hiding this comment.
we should promote this to a with statement:
with HDF5(h5file) as hdf5_instance:
io_manager = IOManager(hdf5_instance)
main(io_manager, saveall=saveall, jupyterlab=compress)this handles opening and closing all in one. even better would be to wrap this all in the IO manager, and have that be the context manager.
I'm pretty sure this edit would work without changing any of the hsp2io.hdf code too, it seems to already be written to be used as a context manager like this.
There was a problem hiding this comment.
Heard and implemented. This functions fine and passes tests, and also, as you note avoids a stuck file if something catastrophic happens during runtime.
Closed in favor of cleaner git log in #217