-
-
Notifications
You must be signed in to change notification settings - Fork 329
Remove old v3 #1742
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
Remove old v3 #1742
Conversation
@d-v-b - do you think this code will ever be released? Since your target is the v3 branch, I think its reasonable to just remove these sections now. |
* refactor(v3): Using appropriate types * fix(v3): Typing fixes + minor code fixes * fix(v3): _sync_iter works with coroutines * docs(v3/store/core.py): clearer comment * fix(metadata.py): Use Any outside TYPE_CHECKING for Pydantic * fix(zarr/v3): correct zarr format + remove unused method * fix(v3/store/core.py): Potential suggestion on handling str store_like * refactor(zarr/v3): Add more typing * ci(.pre-commit-config.yaml): zarr v3 mypy checks turned on in pre-commit
…elopers#1728) * Specify v3 hatch envs using GitHub actions matrix * Update .github/workflows/test-v3.yml Co-authored-by: Joe Hamman <[email protected]> * Update .github/workflows/test-v3.yml Co-authored-by: Joe Hamman <[email protected]> * test on 3.12 too * no 3.12 --------- Co-authored-by: Joe Hamman <[email protected]> Co-authored-by: Joe Hamman <[email protected]>
* black -> ruff + cleanup * format * Preserve git blame * pre-commit fix
…ntributing.rst (zarr-developers#1643) Co-authored-by: Joe Hamman <[email protected]>
This is ready for review. To summarize the changes, because it's a lot of LOC:
|
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.
I did not review every single line here but this seems safe to move forward. Much of the remaining legacy code is likely to be retired as well but this certainly helps clean things up in the meantime.
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.
Let's get this merged 👍
The plan here is to remove the old implementation of Zarr v3 (the old version 3). I think fully removing the old v3 stuff would be a minor accelerator for the new V3 effort because it removes the possibility of confusing the old v3 with the new v3 in the codebase, and it reduces the LOC of the codebase which makes the old code easier to read / test.
At the moment, I have just slathered deprecation warnings atop the old V3 routines, which lets my IDE (vscode) visually indicate which stuff is using those old routines. If we feel OK about this effort, I would go ahead and prune the old v3 classes entirely, and remove the WIP tag from this PR.
closes #1771