Skip to content

Remove ArrayV2 #1857

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

Merged
merged 18 commits into from
May 16, 2024
Merged

Remove ArrayV2 #1857

merged 18 commits into from
May 16, 2024

Conversation

normanrz
Copy link
Member

@normanrz normanrz commented May 8, 2024

This PR removes the ArrayV2 class. The Array class can now take either a ArrayMetadata or ArrayV2Metadata object and perform create, open and save accordingly.
Note, there is still a separate create_v2 method. We might want to merge that into the create method.

The V2 codec pipeline is implemented with the new V2Compressor and V2Filters codecs. They act as wrappers around the compressor and filters of zarr v2.

I also did some light refactorings in the Group to unify with the Array.

cc @jhamman

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)

@normanrz normanrz added the V3 label May 8, 2024
@normanrz normanrz changed the title Implement v2 encode/decode with the codec pipelines Remove ArrayV2 May 9, 2024
@normanrz normanrz requested review from jhamman and d-v-b May 9, 2024 18:42
@normanrz
Copy link
Member Author

normanrz commented May 9, 2024

@jhamman This PR quickly turned into the elimination of the ArrayV2 class. Is there someone already working on porting over the v2 array tests (@d-v-b)?

Co-authored-by: Davis Bennett <davis.v.bennett@gmail.com>
@normanrz normanrz mentioned this pull request May 10, 2024
6 tasks
Copy link
Member

@jhamman jhamman left a comment

Choose a reason for hiding this comment

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

This is great @normanrz! I'll give this a deeper look next week but here are some initial reactions.

@normanrz normanrz requested a review from jhamman May 13, 2024 19:34
@normanrz normanrz requested a review from d-v-b May 13, 2024 20:17
Copy link
Member

@jhamman jhamman left a comment

Choose a reason for hiding this comment

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

Thanks @normanrz!

normanrz and others added 3 commits May 14, 2024 08:24
Co-authored-by: Joe Hamman <joe@earthmover.io>
@normanrz
Copy link
Member Author

This PR depends on #1670. I'll leave this one open until the other is ready.

@normanrz normanrz merged commit 95ae4b6 into batched-codec-pipeline May 16, 2024
9 checks passed
@normanrz normanrz deleted the v3-v2-pipeline branch May 16, 2024 09:51
normanrz added a commit that referenced this pull request May 17, 2024
* merge

* refactors CodecPipelines

* fixes

* adds HybridCodecPipeline

* fixes

* typing

* typing

* consistent naming

* Apply suggestions from code review

Co-authored-by: Davis Bennett <davis.v.bennett@gmail.com>

* encode/decode are batched by default

* use zarr.config for batch_size

* don't use global lru_cache

* removes HybridCodecPipeline

* generic codec classes

* default batch size = 1

* default batch size = 1

* docs

* Update src/zarr/codecs/batched_codec_pipeline.py

Co-authored-by: Joe Hamman <joe@earthmover.io>

* mv batched_codec_pipeline -> pipeline

* Remove ArrayV2 (#1857)

* adds wrapper codecs for the v2 codec pipeline

* encode_chunk_key

* refactor ArrayV2 away

* empty zattrs

* Apply suggestions from code review

Co-authored-by: Davis Bennett <davis.v.bennett@gmail.com>

* unify ArrayMetadata

* abstract ArrayMetadata

* unified Array.create

* use zarr.config for batch_size

* __init__.py aktualisieren

Co-authored-by: Joe Hamman <joe@earthmover.io>

* ruff

---------

Co-authored-by: Davis Bennett <davis.v.bennett@gmail.com>
Co-authored-by: Joe Hamman <joe@earthmover.io>

* merge

---------

Co-authored-by: Davis Bennett <davis.v.bennett@gmail.com>
Co-authored-by: Joe Hamman <joe@earthmover.io>
@normanrz normanrz mentioned this pull request May 17, 2024
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants