Skip to content

Deprecate the experimental v3 implementation #1802

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.

Sign up for GitHub

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 6 commits into from
Apr 22, 2024
Merged

Deprecate the experimental v3 implementation #1802

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
97dc187
deprecate(exp-v3): Add a future warning about the pending removal of …
jhamman Apr 19, 2024
775dccd
ignore warning
jhamman Apr 19, 2024
af56384
add test
jhamman Apr 19, 2024
8a13553
Merge branch 'main' of https://github.com/zarr-developers/zarr-python…
jhamman Apr 22, 2024
834a3e1
reset warnings before test
jhamman Apr 22, 2024
c8e2f12
Merge branch 'main' into dep/experimental-v3-impl
jhamman Apr 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,4 +136,5 @@ filterwarnings = [
"error:::zarr.*",
"ignore:PY_SSIZE_T_CLEAN will be required.*:DeprecationWarning",
"ignore:The loop argument is deprecated since Python 3.8.*:DeprecationWarning",
"ignore:The experimental Zarr V3 implementation in this version .*:FutureWarning",
]
13 changes: 13 additions & 0 deletions zarr/_storage/store.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import abc
import os
import warnings
from collections import defaultdict
from collections.abc import MutableMapping
from copy import copy
Expand All @@ -22,9 +23,21 @@
DEFAULT_ZARR_VERSION = 2

v3_api_available = os.environ.get("ZARR_V3_EXPERIMENTAL_API", "0").lower() not in ["0", "false"]
_has_warned_about_v3 = False # to avoid printing the warning multiple times
Copy link
Member

Choose a reason for hiding this comment

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

I thought this was the default but 🤷🏽

https://docs.python.org/3/library/warnings.html#the-warnings-filter

Copy link
Member Author

Choose a reason for hiding this comment

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

I certainly tried this first. It's possible this would have worked if I had put warning elsewhere, but as implemented, the warning is emitted multiple times without this switch. I think this is due to the fact that assert_zarr_v3_api_available is used in various places around the library.



def assert_zarr_v3_api_available():
# we issue a warning about the experimental v3 implementation when it is first used
global _has_warned_about_v3
if v3_api_available and not _has_warned_about_v3:
warnings.warn(
"The experimental Zarr V3 implementation in this version of Zarr-Python is not "
Copy link
Member

@joshmoore joshmoore Apr 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

❤️

(TIL emoji names are case sensitive)

"in alignment with the final V3 specification. This version will be removed in "
"Zarr-Python 3 in favor of a spec compliant version.",
FutureWarning,
stacklevel=1,
)
_has_warned_about_v3 = True
if not v3_api_available:
raise NotImplementedError(
"# V3 reading and writing is experimental! To enable support, set:\n"
Expand Down