BUG: numpy.einsum indexing arrays now accept numpy int type#16080

Merged

seberg merged 8 commits intonumpy:masterfrom

rlintott:einsum-indexing-int-test-fix

May 16, 2020

Contributor

rlintott commented Apr 26, 2020 •

edited

Loading

Fixes #15961.

All current tests pass. I didn't write any new tests for this though, but perhaps I should do that?


          BUG: numpy.einsum indexing arrays now accept numpy int type

3fd7b70

See numpy#15961.

eric-wieser reviewed

View reviewed changes

numpy/core/src/multiarray/multiarraymodule.c Outdated Show resolved Hide resolved

rlintott added 3 commits

April 26, 2020 01:19


          BUG: numpy.einsum indexing arrays now accept numpy int type

cadc292

See numpy#15961


          Using PyArray_PyIntAsIntp helper function instead


          TST: add tests for einsum numpy int and bool list subscripts

c8dd453

Added tests to check that einsum accepts numpy int64 types and
rejects bool. Rejecting bools is new behaviour in subscript lists.
I changed ValueError to TypeError on line 2496 in multiarraymodule.c
as it is more appropriate. I also modified einsumfunc.py to have the
same behaviour as in the C file when checking subscript list.
(Reject bools but accept anything else from operator.index())

Contributor Author

rlintott commented Apr 30, 2020 •

edited

Loading

Please let me know what you think of this new commit! I had to change einsumfunc.py as I did not realise there were two implementations until I started running tests. It does reject bools now. I also changed the valueerror in the C implementation to a typeerror as it seemed more appropriate (and it was causing my tests to fail)

eric-wieser reviewed

View reviewed changes

numpy/core/einsumfunc.py Outdated Show resolved Hide resolved

eric-wieser reviewed

View reviewed changes

numpy/core/tests/test_einsum.py Outdated Show resolved Hide resolved

eric-wieser reviewed

View reviewed changes

numpy/core/tests/test_einsum.py Outdated Show resolved Hide resolved


          Applying requested changes

a5d92ec

eric-wieser reviewed

View reviewed changes

numpy/core/einsumfunc.py Outdated Show resolved Hide resolved

eric-wieser reviewed

View reviewed changes

numpy/core/einsumfunc.py Outdated Show resolved Hide resolved


          improve exception handling

603233c

anirudh2290 reviewed

View reviewed changes

Member

anirudh2290 left a comment

Thanks for working on this, overall looks good !

numpy/core/tests/test_einsum.py Show resolved Hide resolved

charris added 00 - Bug 01 - Enhancement component: numpy._core labels

Member

charris commented May 7, 2020

Could you add a release note for this under Improvements.

charris added the 56 - Needs Release Note. label


          release note added

dae1228

anirudh2290 approved these changes

View reviewed changes

seberg reviewed

View reviewed changes

numpy/core/src/multiarray/multiarraymodule.c Outdated Show resolved Hide resolved

seberg reviewed

View reviewed changes

Member

seberg left a comment

Just some nits, I may just commit those suggestions and merge in a day or so. If you have some time, happy if you finish it off though (and please see if moving the error creation looks better)!

numpy/core/src/multiarray/multiarraymodule.c Outdated Show resolved Hide resolved

numpy/core/src/multiarray/multiarraymodule.c Outdated Show resolved Hide resolved

seberg self-requested a review

May 15, 2020 15:40


          applying sebergs suggestions

eb38895

seberg removed the 56 - Needs Release Note. label

Member

seberg commented May 16, 2020

Thanks Ryan, lets put it in before I wonder if it was nicer before ;).

seberg merged commit 3ba84d2 into numpy:master

rlintott deleted the einsum-indexing-int-test-fix branch

May 10, 2021 08:36

toslunar mentioned this pull request

Make einsum accept subscripts in numpy int cupy/cupy#6506

Merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

00 - Bug 01 - Enhancement component: numpy._core