Support for np.roll

[rjenc29]

Initial commit for CI.

[rjenc29]

I think the CI failure is unrelated; I reckon it would pass on retry.

I have implemented the first two (mandatory) arguments.

Let me know what you think if you get a mo.

[codecov-io]

Codecov Report

❗ No coverage uploaded for pull request base (master@62d9a79). Click here to learn what that means.
The diff coverage is 82.6%.

@@            Coverage Diff            @@
##             master    #3563   +/-   ##
=========================================
  Coverage          ?   80.49%           
=========================================
  Files             ?      395           
  Lines             ?    81347           
  Branches          ?     9261           
=========================================
  Hits              ?    65479           
  Misses            ?    14437           
  Partials          ?     1431

[stuartarchibald]

Thanks for the contribution. In general the implementation etc looks good. I've made a few minor comments, once addressed this should be good to merge. Thanks again.

[stuartarchibald]

Whilst implied by lack of axis support, perhaps this should also explicitly mention that only integer value for shift is supported?

[rjenc29]

Done

[stuartarchibald]

this input is duplicated from about 3 lines up.

[rjenc29]

Fixed

[stuartarchibald]

seems like this makes scalar inputs into arrays, but then this is not converted back to a scalar on return? e.g.

from numba import njit
import numpy as np

z = 1
def foo(z):
    ff = np.roll(z, 1)
    return ff

expect = foo(z)
got = njit(foo)(z)

print(type(expect), expect.ndim, expect)
print(type(got), got.ndim, got)
np.testing.assert_allclose(got, expect)

[rjenc29]

Bah - it's a 0D array regardless of shift.

I thought this would be a one-liner but my tests fail depending on what order I run them in, which I appreciate is somewhat suboptimal :)

Here's a simple repro:

from numba import njit
from numba.extending import overload, types
import numpy as np


def foo(a, shift):
    pass


@overload(foo)
def foo_impl(a, shift):

    def foo_impl_0d(a, shift):
        return np.array(a)

    if isinstance(a, (types.Boolean, types.Number)):
        return foo_impl_0d

@njit
def foo_quickly(a, shift):
    return foo(a, shift)


def check_stuff(a_values):
    for idx, a in enumerate(a_values):
        got = foo_quickly(a, 1).item(0)
        assert got is a, 'got {0} but expected {1}'.format(got, a)


if __name__ == '__main__':

    # this passes
    check_stuff([False, True])
    check_stuff([0, 1])

    # comment out the above and un-comment the below: fails
    # check_stuff([0, 1])
    # check_stuff([False, True])

Per the comments, the assertions pass if I run the tests in one order, but fail if I switch the order.

If you inspect the signatures of foo_quickly then the former case ends up with [(bool, int64), (int64, int64)] whilst the latter only has [(int64, int64)]).

Have I made some festive mishap?

[stuartarchibald]

I think this needs #3612

[rjenc29]

I simply swapped the test order - #3612 would mean I didn't have to, but I figured just as easy to order the tests.

[rjenc29]

Thanks vm for the review - I will address these points asap

[stuartarchibald]

Think that this needs a conflict resolve and then perhaps a sprinkling of np.asarray now that that is working?

[rjenc29]

Yep, leave it with me (the same is probably true of most / all of my other PRs). I will ping you once done.