Skip to content

refactor: Separate registry logic from ChunkGrid base class (fixes #3734)#3814

Closed
abishop1990 wants to merge 4 commits intozarr-developers:mainfrom
abishop1990:issue-3734-chunk-grid-registry
Closed

refactor: Separate registry logic from ChunkGrid base class (fixes #3734)#3814
abishop1990 wants to merge 4 commits intozarr-developers:mainfrom
abishop1990:issue-3734-chunk-grid-registry

Conversation

@abishop1990
Copy link
Contributor

Summary

Refactor the ChunkGrid base class to remove registry responsibility.

Changes

  • Remove ChunkGrid.from_dict class method that handled dispatch to subclasses
  • Create new parse_chunk_grid function to handle deserialization and dispatch
  • Update ArrayV3Metadata to use parse_chunk_grid instead of ChunkGrid.from_dict

Benefits

  • Separation of concerns: base class no longer has registry responsibility
  • Consistency: follows the same pattern as parse_chunk_key_encoding
  • Clarity: registry logic is now explicit and isolated
  • Extensibility: new chunk grid types can be registered without modifying base class

Testing

  • All lint checks pass
  • Syntax validation passes
  • Minimal surface area: only 3 files modified

…rr-developers#3734)

- Remove ChunkGrid.from_dict class method that handled registry dispatch
- Create new parse_chunk_grid function to handle deserialization
- Update ArrayV3Metadata to use parse_chunk_grid instead of ChunkGrid.from_dict
- Follows the same pattern as parse_chunk_key_encoding for consistency
- Improves separation of concerns by removing registry responsibility from base class
@d-v-b
Copy link
Contributor

d-v-b commented Mar 21, 2026

#3815 (comment)

@abishop1990
Copy link
Contributor Author

Closing: Changelog validation continues to fail despite multiple fixes. The approach needs review before re-opening. Issue #3734 remains open for future work.

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.

2 participants