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

Add eye, zeros, zeros_like, ones, ones_like #183

Merged
merged 6 commits into from Sep 18, 2018

Conversation

Projects
None yet
4 participants
@jcrist
Collaborator

jcrist commented Sep 17, 2018

Add implementations of eye, zeros, and zeros_like, mirroring the equivalent numpy api.

@jcrist jcrist force-pushed the jcrist:creation-functions branch from 5beb33c to ff67350 Sep 17, 2018

@mrocklin

This comment has been minimized.

Collaborator

mrocklin commented Sep 17, 2018

Looks like docstrings need to be spaced slightly differently. Maybe a difference in Numpy version?

@jcrist

This comment has been minimized.

Collaborator

jcrist commented Sep 17, 2018

Grumble. Since NORMALIZE_WHITESPACE only treats whitespace variations as equivalent (and not equivalent with the empty string), there's no way to AFAICT to get get the doctests to work with multiple versions of numpy if they differ in output between

np.array([ 0.0, 0.0])  # and
np.array([0.0, 0.0])

Seems to be only an issue with float arrays, just skipped those tests.

@rgommers

This comment has been minimized.

rgommers commented Sep 17, 2018

The normal approach is to only run doctests for the latest version of numpy. With all the print changes around numpy 1.14, that's the only reasonable thing to do in CI.

@mrocklin

This comment has been minimized.

Collaborator

mrocklin commented Sep 17, 2018

@hameerabbasi

This looks generally complete and in excellent shape. I’ve made a few comments that should increase performance generally.

Also it might be nice to have ones_like with fill values.

If you want more visibility for your work (wink wink) a line in the change log would be nice.

coords = np.stack([n_coords, m_coords])
data = np.ones(data_length, dtype=dtype)
return COO(coords, data=data, shape=(N, M), has_duplicates=False)

This comment has been minimized.

@hameerabbasi

hameerabbasi Sep 17, 2018

Collaborator

Might be nice to add sorted=True.

shape = (shape,)
data = np.empty(0, dtype=dtype)
coords = np.empty((len(shape), 0), dtype=np.intp)
return COO(coords, data=data, shape=shape, has_duplicates=False)

This comment has been minimized.

@hameerabbasi

hameerabbasi Sep 17, 2018

Collaborator

sorted=True

>>> zeros_like(x).todense() # doctest: +NORMALIZE_WHITESPACE
array([[0, 0, 0],
[0, 0, 0]])
"""

This comment has been minimized.

@hameerabbasi

hameerabbasi Sep 17, 2018

Collaborator

sorted=True.

n_coords = m_coords = np.arange(data_length, dtype=np.intp)
coords = np.stack([n_coords, m_coords])
data = np.ones(data_length, dtype=dtype)

This comment has been minimized.

@hameerabbasi

hameerabbasi Sep 17, 2018

Collaborator

Data as a scalar should work here. Takes less memory due to scalar broadcasting being a view.

@hameerabbasi

This comment has been minimized.

Collaborator

hameerabbasi commented Sep 17, 2018

Re the printing changes, yes, since we are highly likely to need changes in upcoming NumPy versions, it’s nice to have them with the newer versions.

This has been a pain point in a PR or two, but I think being on old NumPy docstrings would be worse.

@jcrist jcrist changed the title from Add eye, zeros, zeros_like to Add eye, zeros, zeros_like, ones, ones_like Sep 17, 2018

@jcrist

This comment has been minimized.

Collaborator

jcrist commented Sep 17, 2018

Thanks for the review, I believe all comments have been addressed.

@jcrist jcrist force-pushed the jcrist:creation-functions branch from 8db1e2f to 512ed57 Sep 17, 2018

@hameerabbasi

This comment has been minimized.

Collaborator

hameerabbasi commented Sep 18, 2018

Thabks for making these changes! They’re very welcome. Merging now.

@hameerabbasi hameerabbasi merged commit 8da1a35 into pydata:master Sep 18, 2018

5 checks passed

LGTM analysis: Python No alert changes
Details
ci/circleci Your tests passed on CircleCI!
Details
codecov/patch 100% of diff hit (target 97.43%)
Details
codecov/project 97.5% (+0.07%) compared to 8872e09
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment