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

Shape and chunks accept float values when creating an array #2530

Closed
faymanns opened this issue Dec 3, 2024 · 9 comments
Closed

Shape and chunks accept float values when creating an array #2530

faymanns opened this issue Dec 3, 2024 · 9 comments
Labels
bug Potential issues with the zarr-python library V2 Affects the v2 branch

Comments

@faymanns
Copy link
Contributor

faymanns commented Dec 3, 2024

Zarr version

v2.18.3

Numcodecs version

v0.12.1

Python Version

v3.11.2

Operating System

Linux

Installation

Using pip into virtual environment

Description

When you create a new array (e.g. using zarr.zeros), the tuples for shape and chunks can contain float values.
The values after the decimal point are trimmed without a warning.
Is this the intended behavior?
I would expect zarr-python to raise an exception or at least a warning.

Steps to reproduce

import zarr

z = zarr.zeros((100.4, 100.9), chunks=(10.5, 10.1), dtype="i4")
print(z.shape)
print(z.chunks)

As you can see from the output the float values are trimmed without warning:

(100, 100)
(10, 10)

Additional output

No response

@faymanns faymanns added the bug Potential issues with the zarr-python library label Dec 3, 2024
@d-v-b
Copy link
Contributor

d-v-b commented Dec 3, 2024

Thanks for the bug report; we are working on version 3 of zarr-python so I don't think we are going to make any more releases in the 2.x series. I did test this in zarr-python==3.0.0b2:

Shape fails as expected:

>>> zarr.ones((10, 10.0))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/bennettd/.pyenv/versions/3.11.9/lib/python3.11/site-packages/zarr/api/synchronous.py", line 264, in ones
    return Array(sync(async_api.ones(shape, **kwargs)))
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/bennettd/.pyenv/versions/3.11.9/lib/python3.11/site-packages/zarr/core/sync.py", line 141, in sync
    raise return_result
  File "/Users/bennettd/.pyenv/versions/3.11.9/lib/python3.11/site-packages/zarr/core/sync.py", line 100, in _runner
    return await coro
           ^^^^^^^^^^
  File "/Users/bennettd/.pyenv/versions/3.11.9/lib/python3.11/site-packages/zarr/api/asynchronous.py", line 1037, in ones
    return await create(shape=shape, fill_value=1, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/bennettd/.pyenv/versions/3.11.9/lib/python3.11/site-packages/zarr/api/asynchronous.py", line 907, in create
    return await AsyncArray.create(
           ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/bennettd/.pyenv/versions/3.11.9/lib/python3.11/site-packages/zarr/core/array.py", line 458, in create
    shape = parse_shapelike(shape)
            ^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/bennettd/.pyenv/versions/3.11.9/lib/python3.11/site-packages/zarr/core/common.py", line 145, in parse_shapelike
    raise TypeError(msg)
TypeError: Expected an iterable of integers. Got (10, 10.0) instead.

But we do accept a float for chunks, which we should fix:

>>> zarr.ones((10,10), chunks=(2, 2.0))
<Array memory://4409673920 shape=(10, 10) dtype=float64>

@faymanns
Copy link
Contributor Author

faymanns commented Dec 4, 2024

Thanks for the reply @d-v-b! I'd be happy to submit a pull request for the chunks following the way the shape input is parsed.
I guess parsing the input for chunks and chunk_shape with parse_shapelike inside create should be sufficient?

@d-v-b
Copy link
Contributor

d-v-b commented Dec 4, 2024

yes that sounds like the right direction.

@dstansby dstansby added the V2 Affects the v2 branch label Dec 5, 2024
@dstansby
Copy link
Contributor

dstansby commented Dec 5, 2024

Thanks for the bug report; we are working on version 3 of zarr-python so I don't think we are going to make any more releases in the 2.x series

@d-v-b I thought we were supporting verseion 2 for (at least) six months after release of version 3? So I think it would be reasonable to fix this in the version 2 branch, and I'd be happy to review a PR to version 2 and do a bugfix release.

@d-v-b
Copy link
Contributor

d-v-b commented Dec 5, 2024

@dstansby that makes sense! In that case we should fix 2.x and 3.x

@faymanns
Copy link
Contributor Author

faymanns commented Dec 6, 2024

My PR for the main branch has been merged. Should I submit another PR for the v2 branch before closing this issue?

@dstansby
Copy link
Contributor

dstansby commented Dec 6, 2024

A PR to the v2 branch would be great - I would say that instead of raising an error, we should raise a warning on the v2 branch because an error would technically be a breaking change.

@faymanns
Copy link
Contributor Author

Sorry that it took me so long to do this. I created a pull request (#2579) for v2 that raises warnings.
@dstansby could you take a look at it? I didn't find a logger in v2 so I used the warnings module.

@faymanns
Copy link
Contributor Author

faymanns commented Jan 8, 2025

Fixed for v3 in #2535 and fixed for v2 in #2579.

@faymanns faymanns closed this as completed Jan 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Potential issues with the zarr-python library V2 Affects the v2 branch
Projects
None yet
Development

No branches or pull requests

3 participants