Cleanup and modernize dask.array creation functions (e.g. da.ones)#6276
Open
Cleanup and modernize dask.array creation functions (e.g. da.ones)#6276
Conversation
These are some of the oldest methods in `dask.array`, and stylistically no longer matched much of the codebase. The implementation was also overcomplicated to support `ones` and `ones_like` with the same wrapper code, even though we no longer used the `ones_like` defined in that file (that had since moved to `da.creation` from `da.wrap`). The wrapping code also didn't properly handle the differences between `*args` and `**kwargs` well, leading to issues if e.g. `dtype` was passed as an `arg` not a `kwarg`. This fixes all of that, removing the old code in `wrap`, and moving everything under `da.creation`. `da.ones`/`da.zeros`/`da.full`/`da.empty` all have proper signatures now, and the underlying implementation is much simpler. Also fixes a bug where calling `da.full(large_shape, 1)` with no `dtype` would actually create that array in memory when inferring the dtype.
Member
|
Haven't looked closely, but will this conflict with #6129? |
Member
Author
|
There will be merge conflicts, but it doesn't go against what's done in that PR. |
2 tasks
2 tasks
Member
|
Note that there are also changes in #6064 that will make more conflicts for this PR. |
Member
|
ping @jcrist on cleaning up the conflicts here |
bmerry
reviewed
Feb 8, 2021
| def full(shape, fill_value, dtype=None, chunks="auto", **kwargs): | ||
| if dtype is None: | ||
| dtype = np.array(fill_value).dtype | ||
| return _handle_creation(np.full, shape, dtype, fill_value, chunks=chunks) |
Contributor
There was a problem hiding this comment.
Should these functions be forwarding their kwargs to _handle_creation?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
These are some of the oldest methods in
dask.array, and stylisticallyno longer matched much of the codebase. The implementation was also
overcomplicated to support
onesandones_likewith the same wrappercode, even though we no longer used the
ones_likedefined in that file(that had since moved to
da.creationfromda.wrap). The wrappingcode also didn't properly handle the differences between
*argsand**kwargswell, leading to issues if e.g.dtypewas passed as anargnot akwarg.This fixes all of that, removing the old code in
wrap, and movingeverything under
da.creation.da.ones/da.zeros/da.full/da.emptyall have proper signaturesnow, and the underlying implementation is much simpler.
Also fixes a bug where calling
da.full(large_shape, 1)with nodtypewould actually create that array in memory when inferring the dtype.
Fixes #6275.
black dask/flake8 dask