-
Notifications
You must be signed in to change notification settings - Fork 5
Refactor to incorporate latest py-hamt changes #17
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
|
|
||
| return data | ||
|
|
||
| def __setitem__(self, key: str, value: bytes): |
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.
just curious here, if the code is set to read_only, it seems like it will still write just without a lock. maybe it should be
if self.read_only return
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.
You're right Phil, this missed my notice, I'll do like in py-hamt and raise an exception if a read only ipldstore is being modified
| shutil.rmtree(temp_dir) | ||
|
|
||
|
|
||
| def test_upload_then_read(random_zarr_dataset: tuple[str, xr.Dataset]): |
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.
maybe we add a read only test
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.
Sounds good, I'll add some pytest.with_exceptions and then try to modify the ipldstore
|
This PR is being closed since py-hamt, which ipldstore wraps over, is a MutableMapping and now has testing to guarantee that it works on its own as a zarr backing store. See this PR dClimate/py-hamt#7 |
This incorporates the latest changes from py-hamt, which made for significant changes to the library since the library previously had to wrap over unsupported features of the hamt. This PR also updates the project environment tooling to use uv.
This is currently a draft pull request since documentation through pdoc has still to be added, but the code itself is ready to review and be tested. There is 100% code coverage.
CAR support was previously unsupported and it still is unimplemented in this PR, but we will leave this as a future PR to work off of this new base.