Conversation
|
This looks good to me. The test failure is I suspect due to an upstream dependency shift. It looks like I've just pushed #3723 |
|
Same problem here as in the other build - https://travis-ci.org/dask/dask/jobs/400540686#L1337-L1352 |
|
Yeah, my guess is that the recent update of pillow caused something odd to happen. We should probably report this upstream. |
|
The CI failure has been patched up for now. I believe that you may have to merge with master though. |
dask/array/core.py
Outdated
| chunks = ((0,),) * len(shape) | ||
|
|
||
| if (shape and len(shape) == 1 and len(chunks) > 1 and | ||
| all(type(c) is not tuple for c in chunks)): |
There was a problem hiding this comment.
Would it be ok to replace type(c) is not tuple with isinstance(c, numbers.Number)? That way we'll probably continue doing something sensible if they pass a list like chunks=[[1], [2], [3], [4]]. Obviously this isn't recommended, but new users can do all sorts of things :)
There was a problem hiding this comment.
[[1], [2], [3], [4]] is already covered by the unit tests. Anyway I changed it.
| with pytest.raises(ValueError): | ||
| normalize_chunks(((10,), ), (11, )) | ||
| with pytest.raises(ValueError): | ||
| normalize_chunks(((5, ), (5, )), (5, )) |
There was a problem hiding this comment.
Thanks for the extra tests here
| from dask.array.core import ( | ||
| normalize_chunks as _normalize_chunks, | ||
| ) | ||
| from dask.array.core import normalize_chunks |
There was a problem hiding this comment.
Yeah, that makes sense to me. Thanks
|
This seems good to me. I had one minor comment, but would be happy ignoring it as well. |
|
@mrocklin ready for merge |
|
Thanks @crusaderky ! Merging. |
2 participants
Closes #3719.
The PR is complete for the specific trivial problem. However I'd also like to change normalize_chunks to prevent similar problems in the future - see discussion in #3719.