-
Notifications
You must be signed in to change notification settings - Fork 5
fix: cleanup temporary files #143
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
`tempfile.mktemp()` does not clean up after itself
|
For pytest, you want to use the fixtures for this. I’ll help |
|
OK, here we go. Ended up being a major restructuring, since to do this right, we need to override the But now all the scoping is done by pytest, so it cleans up, and the arrays aren’t created in pytests’ collection phase anymore (should speed that up by a lot and therefore help with IDE performance as well) |
ilan-gold
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome paradigm to override, I have found myself in this tricky situation of param dependencies before
|
Yeah, that’s how pytest is intended to work. Isaac always wanted flat test file directories (I don’t think I’ve ever heard a reason) so we don’t use it much. |
| ) -> zarr.Array: | ||
| return zarr.create( | ||
| (axis_size_,) * dimensionality, | ||
| store=LocalStore(root=tmp_path / ".zarr"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would in-memory storage also work here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't think so since this package doesn't support that kind of store crossing into rust
tempfile.mktemp()does not clean up after itself, and I could see my/tmpgrowing as I ran a few tests