Adds endpoint and retstep support for linspace#3675
Conversation
|
|
||
| def linspace(start, stop, num=50, chunks=None, dtype=None): | ||
| def linspace(start, stop, num=50, endpoint=True, retstep=False, chunks=None, | ||
| dtype=None): |
There was a problem hiding this comment.
Guess this changes the argument order a little bit. Don't have any insightful thoughts on this ATM, but just checking to see if we should consider doing anything else or more here (warning, deprecation cycle, or nothing).
There was a problem hiding this comment.
Good catch @jakirkham, that was an oversight on my part. I'd be happy to change the order here so that it doesn't break any existing code. However, if we are okay changing the order, I'd be in favor of having the same order as np.linspace followed by chunks. I.e.
def linspace(start, stop, num=50, endpoint=True, retstep=False, dtype=None, chunks=None):There was a problem hiding this comment.
Yeah, I kind of like the order you have chosen as well. Moving chunks to the end seems fine to me (assuming we are ok with breaking the order). Let's see if others have any comments on this. Don't expect too much controversy as these are intended to be keyword arguments.
dask/array/creation.py
Outdated
| If True, ``stop`` is the last sample. Otherwise, it is not included. | ||
| Default is True. | ||
|
|
||
| .. versionadded:: 0.18.2 |
There was a problem hiding this comment.
Are we planning on another patch release, @mrocklin?
There was a problem hiding this comment.
I'll remove the versionadded since we're unsure about the next release version
dask/array/creation.py
Outdated
| blockstart = blockstart + (space * bs) | ||
| bs_space = bs - 1 if endpoint else bs | ||
| blockstop = blockstart + (bs_space * step) | ||
| task = (partial(np.linspace, endpoint=endpoint, dtype=dtype), blockstart, blockstop, bs) |
There was a problem hiding this comment.
Might need to wrap this line.
| bs_space = bs - 1 if endpoint else bs | ||
| blockstop = blockstart + (bs_space * step) | ||
| task = (partial(np.linspace, endpoint=endpoint, dtype=dtype), blockstart, blockstop, bs) | ||
| blockstart = blockstart + (step * bs) |
There was a problem hiding this comment.
Is this needed for something?
There was a problem hiding this comment.
It's needed to update the start parameter for np.linspace that's being applied to each block
|
Looks pretty good. Thanks @jrbourbeau. Couple minor comments above. |
|
LGTM. Will merge tomorrow end-of-day if no comments. |
|
Thanks @jrbourbeau :) |
….com/convexset/dask into fix-tsqr-case-chunk-with-zero-height * 'fix-tsqr-case-chunk-with-zero-height' of https://github.com/convexset/dask: fixed typo in documentation and improved clarity Implement .blocks accessor (dask#3689) Fix wrong names (dask#3695) Adds endpoint and retstep support for linspace (dask#3675) Add the @ operator to the delayed objects (dask#3691) Align auto chunks to provided chunks, rather than shape (dask#3679) Adds quotes to source pip install (dask#3678) Prefer end-tasks with low numbers of dependencies when ordering (dask#3588) Reimplement argtopk to release the GIL (dask#3610) Note `da.pad` can be used with `map_overlap` (dask#3672) Allow tasks back onto ordering stack if they have one dependency (dask#3652) Fix extra progressbar (dask#3669) Break apart uneven array-of-int slicing to separate chunks (dask#3648) fix for `dask.array.linalg.tsqr` fails tests (intermittently) with arrays of uncertain dimensions (dask#3662)
This PR adds support for the
endpointandretstepkeyword arguments toda.linspace. Fixes #3673.flake8 dask