Skip to content

Conversation

jhamman
Copy link
Member

@jhamman jhamman commented Aug 9, 2024

This PR creates a core subpackage in the zarr package and begins to move some modules there. As I explained in #2037 (comment), this is designed to clarify which parts of the zarr project are public API and which are internal (everything in core).

In my in my initial commit, I only moved array.py, config.py, group.py, indexing.py, and sync.py. More to be done but I wanted to put this up for an initial reactions before going through the complete set of modules.

closes #2037

Copy link
Member

@normanrz normanrz left a comment

Choose a reason for hiding this comment

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

Lgtm

Copy link
Contributor

@dstansby dstansby left a comment

Choose a reason for hiding this comment

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

As a general comment, objects should only be listed in one all throughout the codebase. This should be where we are expecting users to import the object from. We are still free to import from other locations internally, but all signals where the public API lives.

@jhamman jhamman requested a review from dstansby August 12, 2024 22:36
@jhamman
Copy link
Member Author

jhamman commented Aug 12, 2024

@zarr-developers/python-core-devs - this is ready for a real review now. Thanks @dstansby and @normanrz for an initial look. With these changes, the package organization now looks like this:

.
├── __init__.py
├── abc
│   ├── __init__.py
│   ├── codec.py
│   ├── metadata.py
│   └── store.py
├── api
│   ├── __init__.py
│   ├── asynchronous.py
│   └── synchronous.py
├── codecs
│   ├── __init__.py
│   ├── _v2.py
│   ├── blosc.py
│   ├── bytes.py
│   ├── crc32c_.py
│   ├── gzip.py
│   ├── pipeline.py
│   ├── registry.py
│   ├── sharding.py
│   ├── transpose.py
│   └── zstd.py
├── convenience.py
├── core
│   ├── __init__.py
│   ├── array.py
│   ├── array_spec.py
│   ├── attributes.py
│   ├── buffer.py
│   ├── chunk_grids.py
│   ├── chunk_key_encodings.py
│   ├── common.py
│   ├── config.py
│   ├── group.py
│   ├── indexing.py
│   ├── metadata.py
│   └── sync.py
├── creation.py
├── errors.py
├── py.typed
├── registry.py
├── store
│   ├── __init__.py
│   ├── _utils.py
│   ├── common.py
│   ├── local.py
│   ├── memory.py
│   └── remote.py
├── strategies.py
├── testing
│   ├── __init__.py
│   ├── buffer.py
│   ├── store.py
│   └── utils.py

@jhamman jhamman changed the title create core subpackage and move initial modules [v3] reorganize package - move most modules to zarr.core Aug 12, 2024
@jhamman jhamman added the V3 label Aug 13, 2024
@jhamman jhamman added this to the 3.0.0 milestone Aug 13, 2024
Copy link
Contributor

@dstansby dstansby left a comment

Choose a reason for hiding this comment

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

LGTM! I left some optional suggestions to move the __all__ to the tops of files.

Once this is in it should unblock #2002, which will make it easier to see what the documented public API looks like 😄

Copy link
Contributor

@d-v-b d-v-b left a comment

Choose a reason for hiding this comment

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

I like it! I do think we should move the __all__ declarations to the top of each file as per @dstansby's suggestion, but other than that I think it's good to go

@d-v-b
Copy link
Contributor

d-v-b commented Aug 14, 2024

merging! thanks @jhamman

@d-v-b d-v-b merged commit 10ae5f3 into zarr-developers:v3 Aug 14, 2024
25 checks passed
dcherian added a commit to dcherian/zarr-python that referenced this pull request Aug 14, 2024
* v3:
  [v3] reorganize package - move most modules to zarr.core (zarr-developers#2072)
@jhamman jhamman deleted the feature/core-subpackage branch August 15, 2024 22:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

zarr.{array,config,group} are both modules and functions
4 participants