[WIP] getitems / setitems for BaseStore by d-v-b · Pull Request #1040 · zarr-developers/zarr-python · GitHub
Skip to content

[WIP] getitems / setitems for BaseStore#1040

Closed
d-v-b wants to merge 11 commits into
zarr-developers:mainfrom
d-v-b:getitems_for_all_stores
Closed

[WIP] getitems / setitems for BaseStore#1040
d-v-b wants to merge 11 commits into
zarr-developers:mainfrom
d-v-b:getitems_for_all_stores

Conversation

@d-v-b

@d-v-b d-v-b commented May 26, 2022

Copy link
Copy Markdown
Contributor

This PR adds getitems and setitems methods to BaseStore, and removes some redundant hasattr(store, 'getitems') calls. This makes FSStore a bit less special and hopefully would make async array methods a little easier to engineer.

Tests and docs are WIP

TODO:

  • Add unit tests and/or doctests in docstrings
  • Add docstrings and API docs for any new/modified user-facing classes and functions
  • New/modified features documented in docs/tutorial.rst
  • Changes documented in docs/release.rst
  • GitHub Actions have all passed
  • Test coverage is 100% (Codecov passes)

@pep8speaks

pep8speaks commented May 26, 2022 •
edited
Loading

Copy link
Copy Markdown

@codecov

codecov Bot commented May 26, 2022 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #1040 (7f3e5a4) into main (761bbed) will decrease coverage by 0.17%.
The diff coverage is 100.00%.

❗ Current head 7f3e5a4 differs from pull request most recent head 9f74667. Consider uploading reports for the commit 9f74667 to get more accurate results

@@            Coverage Diff             @@
##             main    #1040      +/-   ##
==========================================
- Coverage   99.94%   99.77%   -0.18%     
==========================================
  Files          34       34              
  Lines       13847    13873      +26     
==========================================
+ Hits        13840    13842       +2     
- Misses          7       31      +24
Impacted Files Coverage Δ
zarr/_storage/store.py 100.00% <100.00%> (ø)
zarr/core.py 96.83% <100.00%> (-3.17%) ⬇️
zarr/storage.py 100.00% <100.00%> (ø)
zarr/tests/test_storage.py 100.00% <100.00%> (ø)

@jakirkham jakirkham mentioned this pull request Jun 2, 2022
Open
@joshmoore

joshmoore commented Jun 23, 2022

Copy link
Copy Markdown
Member

Any chance of getting the codecov back up, @d-v-b ?

@d-v-b

d-v-b commented Jun 23, 2022

Copy link
Copy Markdown
Contributor Author

This is going to take a bit, since it kind of spiraled into a potential refactoring of how slicing works 😰

@jakirkham

jakirkham commented Oct 7, 2022

Copy link
Copy Markdown
Member

Thanks for working on this Davis! 🙏

We are taking another pass at doing this in PR ( #1131 ). If you have time to look, would welcome your feedback there 🙂

@jakirkham

jakirkham commented Jun 22, 2023

Copy link
Copy Markdown
Member

@madsbk , could you please look over this and see if there are things we need to still include from it since PR ( #1131 ) has been merged?

@jakirkham jakirkham mentioned this pull request Jun 22, 2023
Open
@jhamman

jhamman commented Dec 7, 2023

Copy link
Copy Markdown
Member

@d-v-b - what do you think about closing this. The store api is due to change significantly in v3 so I'm not sure this is worth finishing.

@d-v-b

d-v-b commented Dec 7, 2023

Copy link
Copy Markdown
Contributor Author

@d-v-b d-v-b closed this Dec 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@d-v-b @pep8speaks @joshmoore @jakirkham @jhamman