Skip to content
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

Clean up zarr.core.buffer API surface #2641

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

dstansby
Copy link
Contributor

@dstansby dstansby commented Jan 4, 2025

This cleans up the zarr.core.buffer API, by making zarr.core.buffer._core private. Everything was already imported into zarr.core.buffer, but imports elsewhere in the library just needed changing.

@dstansby dstansby added this to the 3.0.0 milestone Jan 4, 2025
@dstansby dstansby mentioned this pull request Jan 4, 2025
@jhamman
Copy link
Member

jhamman commented Jan 6, 2025

We should make sure we don't end up with two levels of underscores. My preference would be we do the full zarr._core switch rather than the sub-sub modules. Before moving forward with these, let's address the broader question of which parts of the zarr.core module need to be available as public API (#2621).

@dstansby dstansby marked this pull request as draft January 6, 2025 10:13
@jhamman
Copy link
Member

jhamman commented Jan 8, 2025

@dstansby - do you have thoughts on how to proceed here given #2669?

I'd be happy to see this land today but I don't think it needs to block 3.0.0.

@jhamman jhamman modified the milestones: 3.0.0, After 3.0.0 Jan 8, 2025
@dstansby
Copy link
Contributor Author

dstansby commented Jan 9, 2025

Yeah, doesn't need to block 3.0.0, and needs updating, but would be good to keep it open to remind me to update it at some point

@dstansby dstansby added the needs release notes Automatically applied to PRs which haven't added release notes label Jan 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs release notes Automatically applied to PRs which haven't added release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants