-
-
Notifications
You must be signed in to change notification settings - Fork 324
Avoid memory copy in local store write #2944
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
Avoid memory copy in local store write #2944
Conversation
thanks! this looks great. |
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.
As the pre-commit
typing failure indicates, doesn't f.write()
need bytes given instead of a numpy array here? Maybe it doesn't since the tests all seem to pass - if that's the case, could you add an appropriate # ignore
comment to fix the mypy error, and add a comment in the code explaining why it's okay to hand a numpy array directly to write()
?
Bumping the python version in our mypy config at Line 352 in 9e8b50a
|
SPEC 0 has support Python 3.11 for a little while yet, so I don't think we should bump the Python version in the config. If it's all fine in practice, then just an ignore comment to make |
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.
Also needs a release note: https://zarr.readthedocs.io/en/stable/developers/contributing.html#changelog
Looks good once those are addressed.
Co-authored-by: Tom Augspurger <[email protected]>
Co-authored-by: Tom Augspurger <[email protected]>
…the buffer protocol
The codecov failure seems to come from increasing the number of lines in an uncovered branch. #2859 is already tracking that, so I'll go ahead and merge this. |
* Avoid memory copy in local store write
This removes an unnecessary memory copy when writing to a local file, bringing the memory usage in line with Zarr Python v2 for this case (see https://github.com./tomwhite/memray-array).
TODO:
docs/user-guide/*.rst
changes/