Skip to content

Feature/store list methods async generators #1782

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.

Sign up for GitHub

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

Jump to bottom
Closed
jhamman wants to merge 52 commits into from
Closed

Feature/store list methods async generators #1782

jhamman wants to merge 52 commits into from

Conversation

@jhamman
Copy link
Member

@jhamman jhamman commented Apr 10, 2024

Putting this PR up for discussion at tomorrow's refactor meeting.

on top of #1743

cc @d-v-b
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)

d-v-b and others added 23 commits March 26, 2024 21:04
@d-v-b
feat: functional .children method for groups
e492be2
@d-v-b
changes necessary for correctly generating list of children
b7f66c7
@d-v-b
add stand-alone test for group.children
c7b333a
@d-v-b
give type hints a glow-up
a64b342
@d-v-b
test: use separate assert statements to avoid platform-dependent orde…
3d11fc0 
…ring issues
@d-v-b
test: put fixtures in conftest, add MemoryStore fixture
cf34afc
@d-v-b
docs: release notes
16cb226
@d-v-b
test: remove prematurely-added mock s3 fixture
b28eaee
@d-v-b
chore: move v3 tests into v3 folder
0215b97
@d-v-b
chore: type hints
e33ca6e
@d-v-b
test: add schema for group method tests
f237172
@d-v-b
chore: add type for zarr_formats
4605446
@d-v-b
chore: remove localstore for now
6e5ded9
@d-v-b
test: add __init__.py to support imports from top-level conftest.py, …
f81b6f3 
…and add some docstrings, and remove redundant def
@d-v-b
fix: return valid JSON from GroupMetadata.to_bytes for v2 metadata
c768ca8
@d-v-b
fix: don't use a type as a value
c405506
@d-v-b
test: add getitem test
5b96554
@d-v-b
fix: replace reference to nonexistent method in with , which does exist
d77d55f
@d-v-b
test: declare v3ness via directory structure, not test file name
05f89c6
@d-v-b
add a docstring to _get, and pass auto_mkdir to _put
b2f9bee
@d-v-b
fix: add docstring to LocalStore.get_partial_values; adjust body of L…
88f4c46 
…ocalStore.get_partial_values to properly handle the byte_range parameter of LocalStore.get.
@d-v-b
test: add tests for localstore init, set, get, get_partial
624ff77
@jhamman
feature(store): make list_* methods async generators
7dff5e5
@pep8speaks
Copy link

pep8speaks commented Apr 10, 2024 •
edited
Loading

Hello @jhamman! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 313:83: W291 trailing whitespace
Line 319:101: E501 line too long (101 > 100 characters)
Line 320:101: E501 line too long (102 > 100 characters)

Line 161:5: E303 too many blank lines (2)
Line 177:5: E303 too many blank lines (2)
Line 192:1: W293 blank line contains whitespace

Line 59:29: E203 whitespace before ':'

Comment last updated at 2024-04-19 03:22:52 UTC

jhamman
jhamman commented Apr 10, 2024
View reviewed changes
src/zarr/v3/group.py Outdated

async with aitercontext(children) as safe_children:
async for child in safe_children:
yield child
Copy link
Member Author

@jhamman jhamman Apr 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@d-v-b - take a look here.

d-v-b added 22 commits April 12, 2024 14:35
@d-v-b
chore: move v3 tests into v3 folder
d264f71
@d-v-b
chore: type hints
0741bed
@d-v-b
test: add schema for group method tests
eb8a535
@d-v-b
chore: add type for zarr_formats
ee2e233
@d-v-b
chore: remove localstore for now
01eec6f
@d-v-b
test: add __init__.py to support imports from top-level conftest.py, …
acae77a 
…and add some docstrings, and remove redundant def
@d-v-b
fix: return valid JSON from GroupMetadata.to_bytes for v2 metadata
ebe1548
@d-v-b
fix: don't use a type as a value
3dce5e3
@d-v-b
test: add getitem test
7f82fdf
@d-v-b
fix: replace reference to nonexistent method in with , which does exist
1655ff8
@d-v-b
test: declare v3ness via directory structure, not test file name
e8514b1
@d-v-b
add a docstring to _get, and pass auto_mkdir to _put
dacacc8
@d-v-b
fix: add docstring to LocalStore.get_partial_values; adjust body of L…
5d2a532 
…ocalStore.get_partial_values to properly handle the byte_range parameter of LocalStore.get.
@d-v-b
test: add tests for localstore init, set, get, get_partial
06d8b04
@d-v-b
fix: remove pre-emptive fetching from group.open
d8749de
@d-v-b
Merge branch 'v3_group_tests' of github.com:d-v-b/zarr-python into v3…
a2fe4e0 
…_group_tests
@d-v-b
fix: use removeprefix (removes a substring) instead of strip (removes…
eed03c8 
… any member of a set); comment out / avoid tests that cannot pass right now; don't consider implicit groups for v2; check if prefix is present in storage before opening for Group.getitem
@d-v-b
xfail v2 tests that are sure to fail; add delitem tests; partition xf…
75f75b1 
…ailing tests into subtests
@d-v-b
fix: handle byte_range[0] being None
8a14e3b
@d-v-b
fix: adjust test for localstore.get to check that get on nonexistent …
459bb42 
…keys returns None; correctly create intermediate directories when preparing test data in test_local_store_get_partial
@d-v-b
fix: add zarr_format parameter to array creation routines (which rais…
8ef3fec 
…es if zarr_format is not 3), and xfail the tests that will hit this condition. add tests for create_group, create_array, and update_attributes methods of asyncgroup.
@d-v-b
test: add group init test
b5a7698
@jhamman jhamman force-pushed the feature/store-list-methods-async-generators branch from 94b00c7 to 7dff5e5 Compare April 19, 2024 03:05
@jhamman
Copy link
Member Author

jhamman commented Apr 19, 2024

closing in favor of d-v-b#110

@jhamman jhamman closed this Apr 19, 2024
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.

3 participants

@jhamman @pep8speaks @d-v-b