use the JSON form of the fill value #77

d-v-b · 2025-07-16T16:21:24Z

...instead of array.fill_value.

d-v-b · 2025-07-16T16:21:43Z

closes #75

…examples

d-v-b · 2025-07-16T16:48:28Z

I just pushed a commit that generates a bunch of zarr data types to use in testing from_zarr. I think it will reveal some interesting failures related to our fill_value annotation

d-v-b · 2025-07-16T17:21:44Z

I fixed the V3 array serialization bugs by allowing the FIllValue field to have type str. Now I'm porting the same v3 tests over to v2, and finding more fill value bugs to fix :)

d-v-b · 2025-07-16T19:54:40Z

v2 fill value bugs were extensive, and should now be fixed. Specifically, we were not handling string fill values, and we were also not handling the zarr v2 identifier for a structured dtype properly.

d-v-b · 2025-07-16T19:58:56Z

this PR expands the set of properly supported data types. it unlocks support for complex floats in zarr v2, fixed-length unicode strings in v2 and v3, raw bytes in v2 and v3, null terminated bytes in v2 and v3, etc

… into fill-value-fix

pyproject.toml

dstansby

Looks good to me. My comments are mainly about improving comments to leave a bit more internal documentation about what's going on for future developers looking at the code.

dstansby · 2025-07-22T08:13:02Z

pyproject.toml

+    "ignore:The data type:FutureWarning",
+    "ignore:The codec:UserWarning"


It would be worth expanding the messages here a bit to give a bit more context on the warnings being ignored.

done in 211fd4c

src/pydantic_zarr/core.py

src/pydantic_zarr/v2.py

src/pydantic_zarr/v3.py

tests/test_pydantic_zarr/conftest.py

pyproject.toml

… into fill-value-fix

d-v-b · 2025-07-24T17:12:41Z

I think we can release as soon as this is in

dstansby · 2025-07-25T19:10:35Z

Could we make sure ome-zarr-models works fine first before we release, as at least one external check to pick up errors? I'm already testing at ome-zarr-models/ome-zarr-models-py#209, and it looks like it will be fine, but I think it's worth a cross check to catch any easy-to-find bugs before doing a full release.

d-v-b · 2025-07-25T19:26:28Z

Could we make sure ome-zarr-models works fine first before we release, as at least one external check to pick up errors? I'm already testing at ome-zarr-models/ome-zarr-models-py#209, and it looks like it will be fine, but I think it's worth a cross check to catch any easy-to-find bugs before doing a full release.

definitely!

d-v-b · 2025-07-28T08:02:13Z

I'd like to merge this today @dstansby any objections?

dstansby

I have some pending review comments that I'll submit - it generally looks good, but my main worry is importing private API. This has bitten me big time in the past, and I think it should be avoided at all costs, especially since we have the power to make it public somewhere in zarr-python.

pyproject.toml

dstansby · 2025-07-25T19:12:54Z

src/pydantic_zarr/core.py


 if TYPE_CHECKING:
-    from zarr.abc.store import Store
+    import zarr
+    from zarr.storage._common import StoreLike


Big 👎 for importing private API. Is there somewhere else this can be imported from? If not, this should be made public over in zarr-python first.

I get the sentiment but lets consider a few things here:

Adding public API to zarr python is potentially slow.

pydantic-zarr is pre-1.0, so there is no stability guarantee. We should feel enabled to move extremely quickly here.

We also know when zarr-python will add / remove API, because we are zarr python devs.

Given these conditions, I don't think using private zarr python API should be a blocker for us. We should just use what we need to use, make an issue about it here and in zarr python, and keep developing this package

src/pydantic_zarr/core.py

dstansby · 2025-07-25T19:16:12Z

src/pydantic_zarr/v2.py


    def to_zarr(
        self,
        store: Store,
        path: str,
        *,
        overwrite: bool = False,
-        **kwargs: Any,
+        config: ArrayConfigParams | None = None,


This is a breaking change - so it's not forgotton, can you add a changelog entry for it in this PR.

we don't have a changelog, but I will ensure that this change is noted when we generate release notes in the github release

src/pydantic_zarr/v2.py

dstansby · 2025-07-28T08:33:39Z

Thought I'd post my pending comments in case helpful - I probably won't have time to review thoroughly today though, so no objections to going ahead and merging if you're happy 👍

d-v-b · 2025-07-28T10:00:58Z

thanks @dstansby!

use the JSON form of the fill value

18db83e

add a test fixture for zarr data types, and generate lots of failing …

19826c3

…examples

d-v-b added 3 commits July 16, 2025 18:52

ignore an additional warning

a7a6f47

allow fill_value to be a string

e4f0104

use dtype examples for v2 array tests

d0f53b4

d-v-b added 2 commits July 16, 2025 19:22

remove type:ignores

d09eb18

fix v2 fill value errors

fc1abde

update example code in docs

453bb55

d-v-b marked this pull request as ready for review July 16, 2025 19:57

d-v-b requested a review from dstansby July 16, 2025 19:57

d-v-b added 6 commits July 17, 2025 11:39

fix zarr version parametrization, reveal bugs

81c8aa4

handle more warnings in pyproject.toml

8cb23b9

handle pre-zarr 3.1.0

91382a6

allow fill value to be None, and use janky JSON serialization

0ebe686

update dtype tests

94d07ae

fix inequality direction

c9d1400

d-v-b requested review from dstansby and removed request for dstansby July 17, 2025 13:27

d-v-b added 3 commits July 22, 2025 09:43

allow basemodel in TBAseAttr

6f36e37

fix v2 fill value errors

fdf75c4

Merge branch 'main' of https://github.com/zarr-developers/pydantic-zarr…

e98ed0c

… into fill-value-fix

d-v-b commented Jul 22, 2025

View reviewed changes

pyproject.toml Show resolved Hide resolved

dstansby reviewed Jul 22, 2025

View reviewed changes

d-v-b added 2 commits July 23, 2025 19:14

rename dtype parsing routines to more accurately reflect what they do

e4ef995

Merge branch 'main' of https://github.com/zarr-developers/pydantic-zarr…

c636029

… into fill-value-fix

d-v-b added 9 commits July 24, 2025 17:45

Merge branch 'main' of https://github.com/zarr-developers/pydantic-zarr…

dde814c

… into fill-value-fix

remove comment

27f84ab

remove comment

76a32ab

use model_validate

bce8f09

use create_array instead of create, and use keyword arguments explicitly

68bf6b7

add v3 model like

879450a

replace contains_array and contains_group with maybe_node

0c04143

use maybe_node

caa5a43

make warnings more literate

211fd4c

d-v-b requested a review from dstansby July 24, 2025 17:12

dstansby reviewed Jul 28, 2025

View reviewed changes

d-v-b added 3 commits July 28, 2025 10:37

push imports to the top

b9f00c5

add explanatory comment

74573e6

push warning filters down

acef3be

d-v-b added 4 commits July 28, 2025 13:45

add fill values to dtype examples test fixtures

e46db6a

add env listing step to ci tests

69a66bf

remove whitespace

36e8577

adapt tests to new dtype fixture

016b386

d-v-b merged commit 24094d6 into zarr-developers:main Jul 28, 2025
8 checks passed

d-v-b deleted the fill-value-fix branch July 28, 2025 13:01

		"ignore:The data type:FutureWarning",
		"ignore:The codec:UserWarning"

use the JSON form of the fill value #77

use the JSON form of the fill value #77

Uh oh!

Conversation

d-v-b commented Jul 16, 2025

Uh oh!

d-v-b commented Jul 16, 2025

Uh oh!

d-v-b commented Jul 16, 2025

Uh oh!

d-v-b commented Jul 16, 2025

Uh oh!

d-v-b commented Jul 16, 2025

Uh oh!

d-v-b commented Jul 16, 2025

Uh oh!

Uh oh!

dstansby left a comment

Choose a reason for hiding this comment

Uh oh!

dstansby Jul 22, 2025

Choose a reason for hiding this comment

Uh oh!

d-v-b Jul 24, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

d-v-b commented Jul 24, 2025

Uh oh!

dstansby commented Jul 25, 2025

Uh oh!

d-v-b commented Jul 25, 2025

Uh oh!

d-v-b commented Jul 28, 2025

Uh oh!

dstansby left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dstansby Jul 25, 2025

Choose a reason for hiding this comment

Uh oh!

d-v-b Jul 28, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dstansby Jul 25, 2025

Choose a reason for hiding this comment

Uh oh!

d-v-b Jul 28, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dstansby commented Jul 28, 2025

Uh oh!

d-v-b commented Jul 28, 2025

Uh oh!

Uh oh!

Uh oh!