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.

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
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ 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",
]


Expand Down
13 changes: 13 additions & 0 deletions zarr/_storage/store.py
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 @@ -23,9 +24,21 @@
DEFAULT_ZARR_VERSION: 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

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
20 changes: 19 additions & 1 deletion zarr/tests/test_storage_v3.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,18 @@
import inspect
import os
import tempfile
import warnings

import numpy as np
import pytest

import zarr
from zarr._storage.store import _get_hierarchy_metadata, v3_api_available, StorageTransformer
from zarr._storage.store import (
_get_hierarchy_metadata,
assert_zarr_v3_api_available,
v3_api_available,
StorageTransformer,
)
from zarr._storage.v3_storage_transformers import ShardingStorageTransformer, v3_sharding_available
from zarr.core import Array
from zarr.meta import _default_entry_point_metadata_v3
Expand Down Expand Up @@ -668,6 +674,18 @@ def test_top_level_imports():
assert not hasattr(zarr, store_name) # pragma: no cover


def test_assert_zarr_v3_api_available_warns_once():
import zarr._storage.store

zarr._storage.store._has_warned_about_v3 = False
warnings.resetwarnings()
with pytest.warns() as record:
assert_zarr_v3_api_available()
assert_zarr_v3_api_available()
assert len(record) == 1
assert "The experimental Zarr V3 implementation" in str(record[0].message)


def _get_public_and_dunder_methods(some_class):
return set(
name
Expand Down