-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
da.eye
fix for chunks=-1
#7854
da.eye
fix for chunks=-1
#7854
Conversation
Nice minimal fix 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @ncclementi! Overall this looks great. Passing user-provided chunks
through normalize_chunks
is definitely the way to go.
Fortunately normalize_chunks
has some of the same chunksize calculation logic that we've implemented here in da.eye
. So since we're now always calling normalize_chunks
, we can remove some code from da.eye
which is now unnecessary:
diff --git a/dask/array/creation.py b/dask/array/creation.py
index 2e8b0eab..87cc715b 100644
--- a/dask/array/creation.py
+++ b/dask/array/creation.py
@@ -544,18 +544,12 @@ def eye(N, chunks="auto", M=None, k=0, dtype=float):
if not isinstance(chunks, (int, str)):
raise ValueError("chunks must be an int or string")
- chunks = normalize_chunks(chunks, shape=(N, M), dtype=dtype)
- chunks = chunks[0][0]
+ vchunks, hchunks = normalize_chunks(chunks, shape=(N, M), dtype=dtype)
+ chunks = vchunks[0]
+
token = tokenize(N, chunks, M, k, dtype)
name_eye = "eye-" + token
- vchunks = [chunks] * (N // chunks)
- if N % chunks != 0:
- vchunks.append(N % chunks)
- hchunks = [chunks] * (M // chunks)
- if M % chunks != 0:
- hchunks.append(M % chunks)
-
for i, vchunk in enumerate(vchunks):
for j, hchunk in enumerate(hchunks):
if (j - i - 1) * chunks <= k <= (j - i + 1) * chunks:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @ncclementi! Will merge after CI finishes up
Hmm there's an unrelated daily stock test failure (which I'm able to reproduce locally). Given that the error message says
I'll wait a bit and see if the test passes then |
Just merged |
eye
key error #7852black dask
/flake8 dask
/isort dask
Fixes bug in
da.eye
case of chunks=-1.