Skip to content

ENH: __array_function__ support for np.lib, part 1/2 #12116

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

Merged
merged 1 commit into from
Oct 11, 2018

Conversation

shoyer
Copy link
Member

@shoyer shoyer commented Oct 8, 2018

xref #12028

np.lib.arraypad through np.lib.nanfunctions

np.lib.arraypad through np.lib.nanfunctions
@shoyer shoyer force-pushed the array-function-numpy-lib branch from be2b64f to 4141e24 Compare October 8, 2018 19:53
@shoyer shoyer changed the title ENH: __array_function__ support for np.lib ENH: __array_function__ support for np.lib, part 1 Oct 8, 2018
@shoyer shoyer changed the title ENH: __array_function__ support for np.lib, part 1 ENH: __array_function__ support for np.lib, part 1/2 Oct 8, 2018
Copy link
Contributor

@hameerabbasi hameerabbasi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment here about repeated signatures, otherwise all good.

I see that you've omitted a lot of None checks for this PR and I realise it will never dispatch to those, but it might be nice to make it explicit. I'll leave the decision up to you though.

@@ -36,6 +37,11 @@
]


def _ediff1d_dispatcher(ary, to_end=None, to_begin=None):
return (ary, to_end, to_begin)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs a is not None check.

@@ -3294,6 +3405,12 @@ def _ureduce(a, func, **kwargs):
return r, keepdim


def _median_dispatcher(
a, axis=None, out=None, overwrite_input=None, keepdims=None):
return (a, out)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs a None check for out.

@@ -3583,6 +3706,12 @@ def percentile(a, q, axis=None, out=None,
a, q, axis, out, overwrite_input, interpolation, keepdims)


def _quantile_dispatcher(a, q, axis=None, out=None, overwrite_input=None,
interpolation=None, keepdims=None):
return (a, q, out)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs None check for out.

@shoyer
Copy link
Member Author

shoyer commented Oct 9, 2018

I see that you've omitted a lot of None checks for this PR and I realise it will never dispatch to those, but it might be nice to make it explicit.

I opted against this style mostly because it can make the dispatchers significantly longer, e.g.,

def _ediff1d_dispatcher(ary, to_end=None, to_begin=None):
    return (ary, to_end, to_begin)

vs

def _ediff1d_dispatcher(ary, to_end=None, to_begin=None):
    yield ary
    if to_end is not None:
        yield to_end
    if to_begin is not None:
        yield to_begin

If this actually made a difference in speed I would go for it, but I suspect they will actually be quite similar in performance.

@charris charris merged commit 2ae0014 into numpy:master Oct 11, 2018
@charris
Copy link
Member

charris commented Oct 11, 2018

Thanks Stephan.

@charris charris changed the title ENH: __array_function__ support for np.lib, part 1/2 ENH: __array_function__ support for np.lib, part 1/2 Nov 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants