BUG: astype(..., copy=True) doesn't copy on dask #234
Conversation
|
oops, snuck some misc changes in. Going to revert, sorry! |
lithomas1
force-pushed
the
dask-astype-copy
branch
from
1cb10db to
fef8607
Compare
|
cc @crusaderky @ev-br for review |
| @@ -39,7 +39,14 @@ | |||
|
|
|||
| isdtype = get_xp(np)(_aliases.isdtype) | |||
| unstack = get_xp(da)(_aliases.unstack) | |||
| astype = _aliases.astype | |||
|
|
|||
| def astype(x: Array, dtype: Dtype, /, *, copy: bool = True) -> Array: | |||
There was a problem hiding this comment.
There was a problem hiding this comment.
Thanks,
I just raised Notimplemented for now.
There was a problem hiding this comment.
Please ignore it. astype(x, dtype, device=device(x) must work.
tests/test_common.py
Outdated
|
|
||
| @pytest.mark.parametrize("library", wrapped_libraries) | ||
| def test_astype_copy(library): | ||
| # array-api-tests currently doesn't check copy=True | ||
| # makes a copy when dtypes are the same | ||
| # so we check that here | ||
| xp = import_(library, wrapper=True) | ||
| a = xp.asarray([1]) | ||
| b = xp.astype(a, a.dtype, copy=True) | ||
|
|
||
| a[0] = 10 | ||
|
|
||
| assert b[0] == 1 |
There was a problem hiding this comment.
Please move this test to array-api-tests.
There was a problem hiding this comment.
WIP PR here:
data-apis/array-api-tests#335
Co-Authored-By: Guido Imperiale <6213168+crusaderky@users.noreply.github.com>
|
@ev-br this looks ready to be merged to me |
There was a problem hiding this comment.
The sequence of x.astype, then x.copy looks like making two copies to a non-expert; that said I'm ready to believe it's a right dask-specific pattern, and the PR is okay'd by the subject matter expert. Let's roll with this then. Thank you @lithomas1 , @crusaderky
It does make two copies of the dask collection - which is a thin python object. The dask graph wrapped internally is the same. |
dask.array.astype doesn't respect copy=True, so we should copy manually.