Use context managers in test functions#469
Conversation
Elimiinate 32 of 70 of the following warnings emitted while running unit tests: `RuntimeWarning: deallocating CachingFileManager` The remaining 38 appear to all be related to the HDF5VirtualBackend (see zarr-developers#468) Fixes zarr-developers#390
|
This looks great @chuckwondo , but I'm confused as to how it actually prevents the warnings. We do not yet have any implementation of the |
Right, and that's why my changes do not eliminate all of the warnings. Some of them are eliminated because they introduce context managers where xarray datasets (and other things that deal directly with files) are directly used. The rest remain precisely because of what you mention (hence, why I also opened #468), but I added the context managers in these places anyway, for consistency as well as for automatically taking advantage of the necessary changes. Once the other related issues are addressed, the remaining warning should automatically go away as well. If you like, I can attempt to pare down the changes to only the places that eliminate warnings now, rather than including context managers that will only eliminate warnings later, when the other related changes land. |
|
Thank you @chuckwondo , that makes sense. I just wanted to check I wasn't going to merge this without understanding it :) |
2 participants
Eliminate 32 of 70 of the following warnings emitted while running unit tests:
RuntimeWarning: deallocating CachingFileManagerThe remaining 38 appear to all be related to the HDF5VirtualBackend, so they are not addressed in this PR, as they will require some code refactoring, as identified in #468.
Fixes #390