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

Wire up Zarr property tests EAR-1189 #68

Merged
merged 18 commits into from
Nov 15, 2024
Merged
3 changes: 2 additions & 1 deletion .github/workflows/deploy-docs.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ jobs:
git config user.name github-actions[bot]
git config user.email 41898282+github-actions[bot]@users.noreply.github.com

- uses: actions/setup-python@v5
- id: setup-python
uses: actions/setup-python@v5
with:
python-version: 3.x

Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/publish-rust-library.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ jobs:
docker compose up -d minio
- name: Wait for MinIO to be ready
run: |
for i in {1..10}; do
for _ in {1..10}; do
if curl --silent --fail http://minio:9000/minio/health/live; then
break
fi
Expand All @@ -51,7 +51,7 @@ jobs:
run: cargo install --locked cargo-deny

- name: Check
if: matrix.os == 'ubuntu-latest' || github.event_name == 'push'
if: github.event_name == 'push'
run: |
just pre-commit

Expand Down
24 changes: 21 additions & 3 deletions .github/workflows/python-check.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ jobs:

- name: Wait for MinIO to be ready
run: |
for i in {1..10}; do
for _ in {1..10}; do
if curl --silent --fail http://minio:9000/minio/health/live; then
break
fi
Expand All @@ -50,10 +50,10 @@ jobs:
uses: PyO3/maturin-action@v1
with:
working-directory: icechunk-python
target: ${{ matrix.platform.target }}
# target: ${{ matrix.platform.target }}
args: --release --out dist --find-interpreter
sccache: 'true'
manylinux: ${{ matrix.platform.manylinux }} # https://github.com/PyO3/maturin-action/issues/245
# manylinux: ${{ matrix.platform.manylinux }} # https://github.com/PyO3/maturin-action/issues/245

- name: Cargo lint + format
shell: bash
Expand Down Expand Up @@ -81,6 +81,15 @@ jobs:
pip install icechunk['test'] --find-links dist --force-reinstall
ruff check

- name: Restore cached hypothesis directory
id: restore-hypothesis-cache
uses: actions/cache/restore@v4
with:
path: icechunk-python/.hypothesis/
key: cache-hypothesis-${{ runner.os }}-${{ github.run_id }}
restore-keys: |
cache-hypothesis-

- name: pytest
shell: bash
working-directory: icechunk-python
Expand All @@ -90,3 +99,12 @@ jobs:
source .venv/bin/activate
pip install icechunk['test'] --find-links dist --force-reinstall
pytest

# explicitly save the cache so it gets updated, also do this even if it fails.
- name: Save cached hypothesis directory
id: save-hypothesis-cache
if: always()
uses: actions/cache/save@v4
with:
path: icechunk-python/.hypothesis/
key: cache-hypothesis-${{ runner.os }}-${{ github.run_id }}
2 changes: 1 addition & 1 deletion .github/workflows/python-ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ jobs:
docker compose up -d minio
- name: Wait for MinIO to be ready
run: |
for i in {1..10}; do
for _ in {1..10}; do
if curl --silent --fail http://minio:9000/minio/health/live; then
break
fi
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/rust-ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ jobs:

- name: Wait for MinIO to be ready
run: |
for i in {1..10}; do
for _ in {1..10}; do
if curl --silent --fail http://minio:9000/minio/health/live; then
break
fi
Expand Down
19 changes: 13 additions & 6 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ repos:
hooks:
- id: trailing-whitespace
- id: end-of-file-fixer
# - id: check-yaml
- id: check-yaml
exclude: docs/mkdocs.yml
- id: debug-statements
- id: mixed-line-ending
- repo: https://github.com/astral-sh/ruff-pre-commit
Expand All @@ -14,6 +15,17 @@ repos:
- id: ruff-format
- id: ruff
args: ["--fix", "--show-fixes", "icechunk-python/"]
- repo: https://github.com/rhysd/actionlint
rev: v1.6.26
hooks:
- id: actionlint
files: ".github/workflows/"
args: ["-ignore", "SC1090", "-ignore", "SC2046", "-ignore", "SC2086", "-ignore", "SC2129", "-ignore", "SC2155"]
- repo: https://github.com/codespell-project/codespell
# Configuration for codespell is in .codespellrc
rev: v2.3.0
hooks:
- id: codespell
- repo: local
hooks:
- id: format-lint
Expand All @@ -22,10 +34,5 @@ repos:
entry: just pre-commit
language: system
pass_filenames: false
- repo: https://github.com/codespell-project/codespell
# Configuration for codespell is in .codespellrc
rev: v2.3.0
hooks:
- id: codespell

exclude: 'tests/data/.*'
1 change: 1 addition & 0 deletions icechunk-python/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ test = [
"ruff",
"dask",
"distributed",
"hypothesis",
]

[tool.maturin]
Expand Down
51 changes: 51 additions & 0 deletions icechunk-python/tests/test_zarr/test_properties.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import numpy as np
import pytest
from numpy.testing import assert_array_equal

from icechunk import IcechunkStore, StorageConfig

pytest.importorskip("hypothesis")

import hypothesis.strategies as st
from hypothesis import assume, given, settings

from zarr.testing.strategies import arrays, numpy_arrays
Copy link
Contributor

Choose a reason for hiding this comment

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

This is awesome... we should embrace this for the other tests as well in the future


readable_characters = st.characters(
# only use characters within the "Latin Extended-A" subset of unicode
categories=["L", "N", "P"],
max_codepoint=0x017F,
)
_attr_keys = st.text(readable_characters, min_size=1)
_attr_values = st.recursive(
st.none() | st.booleans() | st.text(readable_characters, min_size=1, max_size=5),
lambda children: st.lists(children) | st.dictionaries(_attr_keys, children),
max_leaves=3,
)
simple_attrs = st.none()

icechunk_stores = st.builds(
IcechunkStore.create,
storage=st.builds(StorageConfig.memory, prefix=st.just("prefix")),
read_only=st.just(False),
)


@settings(report_multiple_bugs=False, deadline=None)
@given(data=st.data(), nparray=numpy_arrays(zarr_formats=st.just(3)))
def test_roundtrip(data: st.DataObject, nparray) -> None:
# TODO: support size-0 arrays GH392
assume(nparray.size > 0)
# TODO: fix complex fill values GH391
assume(not np.iscomplexobj(nparray))

zarray = data.draw(
arrays(
stores=icechunk_stores,
arrays=st.just(nparray),
zarr_formats=st.just(3),
# TODO: use all attrs once fixed GH393
attrs=simple_attrs,
)
)
assert_array_equal(nparray, zarray[:])
Loading