Min max on iterables

[rjenc29]

Initial commit for CI. Addresses #3776

[rjenc29]

Well, that was somewhat less successful than I'd hoped.

I thought I'd tweak the overload implementation a little to support iterable types, but this breaks some of the parallel implementations.

Perhaps tweaking the overload implementation isn't such a good idea. Let me know what you think if you get a mo.

[stuartarchibald]

Thanks for starting work on this fix. The code is largely failing because of a mismatch introduced between the typing and implementation signatures. Typing is declared here:

numba/numba/targets/builtins.py

Line 477 in 72241de

def indval_min(indval1, indval2=None):

one implementation has a matching signature:

numba/numba/targets/builtins.py

Line 466 in 72241de

def impl(indval1, indval2=None):

the other does not:

numba/numba/targets/builtins.py

Line 458 in 72241de

def impl(indval1, indval2):

This section of the build log reports the issue, https://travis-ci.org/numba/numba/jobs/501035162#L3792-L3822, feedback on whether this is sufficiently useful is welcome (it's relatively new as a feature).

[rjenc29]

My apologies, it's obvious now you point it out.

It's a very useful feature and I am not sure how the logs could be any more useful / direct in this case - I guess as the test failure came from an unexpected place, I must have assumed something more sinister at work. Please could we just chalk this up to my daftness (there's a pattern developing here...).

Anyway. the last commit is in the right ballpark - I need to sort out the tests and tidy up a bit and then I will un-WIP and invite for a review. Should be in the next few days.

Cheers

[stuartarchibald]

No problem, it's a new thing! It seems like this PR is along the right lines, thanks for continuing to work on it.

[rjenc29]

If you get a mo, could you let me know how you feel about this incarnation, TA!

[stuartarchibald]

Thanks for the patch. The impl looks good, but I wonder if it can be extracted from the current definition scope to improve clarity (see comments).

[stuartarchibald]

Wondering if it's possible to pull these out of this scope which has a non-intuitive signature? Would defining another overload like:

@overload(min)
def iterable_min(iterable):
    if isinstance(iterable, types.IterableType):
        def impl(iterable):
            <details>

work? Could also close over the <, > operator to save duplication too?

[rjenc29]

Done.

[stuartarchibald]

Perhaps vary the tuple value?

[rjenc29]

Done.

[stuartarchibald]

Thanks for the fixes, this seems good. Whilst the impl does not close over the comparison operation, the llvm inliner should manage to inline the declared function with no trouble.