ENH: Automate signature computation for builtin functions #17818

serge-sans-paille · Nov 21, 2020

This improves the compatibility with the inspect module. Otherwise some functions are not compatible with it. For instance we currently have

>>> import numpy as np, inspect
>>> inspect.getfullargspec(np.empty_like)
FullArgSpec(args=[], varargs='args', varkw='kwargs', defaults=None, kwonlyargs=[], kwonlydefaults=None, annotations={})

while the docstring contains more information

>>> help(np.empty_like)
Help on function empty_like in module numpy:

empty_like(...)
    empty_like(prototype, dtype=None, order='K', subok=True, shape=None)

With this patch, one gets

>>> inspect.getfullargspec(np.empty_like)
FullArgSpec(args=['prototype', 'dtype', 'order', 'subok', 'shape'], varargs=None, varkw=None, defaults=(None, 'K', True, None), kwonlyargs=[], kwonlydefaults=None, annotations={})

serge-sans-paille · Nov 21, 2020

tests are missing, and I'm using an internal API of inspect :-/

eric-wieser · Nov 21, 2020

If this is for C docstrings, python has a builtin mechanism for this via docstrings containing --\n\n. Discussion in #8734

serge-sans-paille · Nov 21, 2020

Yeah, but I cannot inject a __text_signature__ in a buitin, and the doc comes from the wrapper :-/

eric-wieser · Nov 22, 2020

but I cannot inject a __text_signature__ in a buitin

You can, you do it in the same way that we currently inject docs into builtins - all that you need is -- after the signature in the docstring.

Note that the problem I ran into last time is that inspect.Signature cannot parse our default values

serge-sans-paille · Nov 23, 2020

Note that the problem I ran into last time is that inspect.Signature cannot parse our default values

Now that validation passes, I can propose to remove the current call to inspect internal API by a builtin-one that matches numpy formatting, which would fix the default argument you're mentioning (I don't get any error with current parser though, but maybe that was a sillent error?). @eric-wieser, what do you suggest?

Require a few signature patching, but nothing terrible See numpy/numpy#17818

serge-sans-paille · Dec 2, 2020

@eric-wieser ?

serge-sans-paille · Dec 11, 2020

gentle ping :-)

rgommers · Jan 2, 2021

@serge-sans-paille could you add to the PR description an example of the change in user-visible behaviour? That would help in reviewing this PR.

rgommers · Jan 2, 2021

Also interested in how it helps Pythran (at least I assume it does?).

serge-sans-paille · Jan 2, 2021

Also interested in how it helps Pythran (at least I assume it does?).

It's actually a corner case I hit while developing https://github.com/serge-sans-paille/penty, a type inference engine for scientific code written in Python. This project is still in early stages, but the goal is to support numpy's implicit type system, including common cases where the array shape is known, and various kind of fancy indexing can be supported.

The inspect module is used here https://github.com/serge-sans-paille/penty/blob/master/tests/pentest.py#L36 and here https://github.com/serge-sans-paille/penty/blob/master/penty/penty.py#L153 to automatically inspect function signature.

rgommers

This seems like a nice thing to do, and probably preferable to editing all docstrings to add --\n\n and relying on Python to do this.

rgommers · Jan 2, 2021

numpy/tests/test_public_api.py

@@ -471,3 +472,18 @@ def check_importable(module_name):
        raise AssertionError("Modules that are not really public but looked "
                             "public and can not be imported: "
                             "{}".format(module_names))
+
+if sys.flags.optimize < 2:


The normal way to write this would be to decorate the test_ function with @pytest.mark.skipif(sys.flags.optimize < 2)

rgommers · Jan 2, 2021

numpy/core/overrides.py

@@ -213,6 +223,9 @@ def decorator(implementation):
        if module is not None:
            public_api.__module__ = module

+        if docs_from_dispatcher and dispatcher.__doc__:


The and dispatcher.__doc__ seems necessary; strangely enough there's unprotected uses of dispatcher.__doc__ a little higher up in array_function_dispatch that don't seem to cause any issues. Unrelated to this PR, but we may want to check that.

rgommers · Jan 2, 2021

numpy/core/overrides.py

+        function.__signature__ = inspect._signature_fromstr(
+            inspect.Signature, function,
+            docstring.strip().split('\n')[0])
+    except ValueError:


This will need except Exception probably, there's other ways this could conceivably fail.

rgommers · Jan 2, 2021

numpy/core/overrides.py

@@ -34,6 +35,15 @@ def set_array_function_like_doc(public_api):
    return public_api


+def add_signature(function, docstring):
+    try:
+        function.__signature__ = inspect._signature_fromstr(


Would indeed be nice to replace this with a vendored copy or something else that's not private API.

JeanKossaifi · Oct 6, 2021

Nice PR - currently inspect.signature fails for builtins, which this PR seems to fix.

serge-sans-paille · Oct 6, 2021

Thanks! That was the goal, but it got a bit obsolete... I'll try to resurrect it -)

JeanKossaifi · Oct 6, 2021

Awesome, thanks! :)

eric-wieser · Oct 9, 2021

Sorry for never replying to the ping. I'm still not clear on what the resolution to this comment is.

BvB93 · Oct 12, 2021

numpy/core/overrides.py

+    objname = obj.__name__
+    if not head.startswith(objname):
+        return None


This will require some minor modification of the ndarray method docstrings, as they use a f"a.{objname}"-pattern for their signatures:

numpy/numpy/core/_add_newdocs.py

Lines 3656 to 3658 in 8c9cf79

add_newdoc('numpy.core.multiarray', 'ndarray', ('ravel',

"""

a.ravel([order])

serge-sans-paille · Oct 12, 2021

New Update I'm super happy with: instead of getting the _text_signature__ from the docstring, get it from the dispatcher prototype, as returned by inspect.signature. This avoids some redundancy, while also avoiding some explicit string extraction / manipulation.

eric-wieser · Oct 12, 2021

That's going to be significantly slower than setting text_signature, sadly

serge-sans-paille · Oct 13, 2021

for the record:

>>> %timeit foo.__name__ + str(inspect.signature(foo)) + '\n--\n\n'
24.4 µs ± 261 ns per loop (mean ± std. dev. of 7 runs, 10000 loops each)

mattip · Oct 13, 2021

numpy/core/overrides.py

@@ -1,6 +1,7 @@
 """Implementation of __array_function__ overrides from NEP-18."""
 import collections
 import functools
+import inspect


Is this needed?

serge-sans-paille · Oct 14, 2021

I don't have a CircleCI account so I cannot see what's the issue there :-/

mattip · Oct 14, 2021

The docs build ends with this error. I don't know how that is connected to this PR, maybe you need to rebase off master?

LANG=C sphinx-build -b html -WT --keep-going -d build/doctrees  -j4 -q source build/html 
1.22.dev0 1.22.0.dev0+1433.gdc8eba9e8
/home/circleci/repo/doc/source/reference/routines.ma.rst:51:<autosummary>:1: WARNING: Inline emphasis start-string without end-string.
/home/circleci/repo/doc/source/reference/routines.ma.rst:51:<autosummary>:1: WARNING: Inline strong start-string without end-string.
docstring of numpy.ma.empty_like:2: WARNING: Inline emphasis start-string without end-string.
docstring of numpy.ma.empty_like:2: WARNING: Inline strong start-string without end-string.
make: *** [Makefile:182: html-build] Error 1

Exited with code exit status 2

You might want to try building the docs locally.

serge-sans-paille · Oct 14, 2021

@mattip : it's autodoc that interacts in bad manner with __text_signature__ delimiter (namely --)
@eric-wieser : given that using the -- modifier + textwrap.dedent is only marginally faster than using inspect.signature, and the latter doesn't interact badly with autodoc and requires less maintenance, would you be ok if I revert to the previous approach ( https://github.com/serge-sans-paille/numpy/tree/feature/provide-signature-for-some-builtins-alt ) ?

mattip · Oct 14, 2021

This affects import time, right? Could you run with -X importtime and compare the times?

serge-sans-paille · Oct 14, 2021

signature approach: cumulated time for the import of numpy.ma is around 5000us
docstring extraction approach: cumulated time for the import of numpy.ma is around 9000us

So basically if we want to extract the signature from the docstring while keeping the docstring easy to maintain, it actually costs more and it's in compatible with autodoc. And information is duplicated between the docstring and the actual signature.

mattip · Oct 14, 2021

See also #10167, which proposes to not use the python-based add_newdocs at runtime at all.

serge-sans-paille · Oct 14, 2021

And for completeness:

signature approach: 68437.1 us average import time over 100 run
textwrap approach: 70469.8 us average import time over 100 run

And because I had to mention it:
manual textwrap approach: 68086.2 us average import time over 100 run

ie. make the docstring looks less nice but not requiring any processing, as in:

    """empty_like(prototype, dtype=None, order='K', subok=True, shape=None)
--

    Return a new array with the same shape and type as a given array.

    Parameters

mattip · Oct 14, 2021

What is the import time on main before this PR?

serge-sans-paille · Oct 15, 2021

68168.8 us in average

eric-wieser · Oct 15, 2021

it's autodoc that interacts in bad manner with __text_signature__ delimiter (namely --)

I don't really understand that; how is autodoc even seeing the --? That's supposed to be swallowed by CPython, and should not be appearing in __doc__ at all.

This improves the compatibility with the inspect module.

serge-sans-paille · Oct 17, 2021

@eric-wieser I cannot tell for sure. that was just an educated guess as it was the only change. I don't quite want to dug further into that one. The performance impact onf startup of current approach looks very low, and from an engineering point of view, it looks easier to maintain. Is that a blocker?

rgommers · Jun 30, 2022

The performance impact on startup of current approach looks very low, and from an engineering point of view, it looks easier to maintain. Is that a blocker?

2 ms total impact (or <1 ms if it's the signature approach) on import time should not be a blocker. This is a correctness/completeness issue that people keep stumbling over. So +1 for the easier to maintain solution here.

rgommers · Jun 30, 2022

Despite the long and complicated discussion, this is actually a small patch that should be mergeable. Would you mind resolving the merge conflicts @serge-sans-paille?

eric-wieser · Jun 30, 2022

numpy/core/multiarray.py

 def can_cast(from_, to, casting=None):
    """
-    can_cast(from_, to, casting='safe')


Doesn't this mean that the generated signature is now going to say that the default for casting is None? In general, the dispatchers use None as the default for everything, even though that's not reflective of their true signature.

mtsokol · Apr 19, 2024

These changes works with the current main.
My question: It only provides signatures to functions in numpy._core.multiarray, right?
It doesn't impact ufuncs from numpy._core.umath? (they also miss signatures)

serge-sans-paille changed the title ~~Automate signature computation for builtin functions~~ [wip] Automate signature computation for builtin functions Nov 21, 2020

serge-sans-paille force-pushed the feature/provide-signature-for-some-builtins branch 2 times, most recently from f84a4fa to a22c8e9 Compare November 21, 2020 20:06

serge-sans-paille force-pushed the feature/provide-signature-for-some-builtins branch from a22c8e9 to 4d9881a Compare November 22, 2020 09:57

serge-sans-paille changed the title ~~[wip] Automate signature computation for builtin functions~~ Automate signature computation for builtin functions Nov 22, 2020

serge-sans-paille force-pushed the feature/provide-signature-for-some-builtins branch 2 times, most recently from ff13987 to 2b9390c Compare November 22, 2020 16:55

charris changed the title ~~Automate signature computation for builtin functions~~ ENH: Automate signature computation for builtin functions Nov 22, 2020

github-actions bot added the 01 - Enhancement label Nov 22, 2020

charris added the component: numpy._core label Nov 22, 2020

serge-sans-paille force-pushed the feature/provide-signature-for-some-builtins branch from 2b9390c to 0df2c9f Compare November 22, 2020 22:07

serge-sans-paille added a commit to serge-sans-paille/penty that referenced this pull request Nov 25, 2020

Support numpy.{empty,ones,zeros}_like

aec5e29

Require a few signature patching, but nothing terrible See numpy/numpy#17818

serge-sans-paille added a commit to serge-sans-paille/penty that referenced this pull request Nov 26, 2020

Support numpy.{empty,ones,zeros}_like

0ba1ebb

Require a few signature patching, but nothing terrible See numpy/numpy#17818

rgommers reviewed Jan 2, 2021

View reviewed changes

Base automatically changed from master to main March 4, 2021 02:05

serge-sans-paille force-pushed the feature/provide-signature-for-some-builtins branch from 0df2c9f to c7f8956 Compare October 9, 2021 13:09

serge-sans-paille force-pushed the feature/provide-signature-for-some-builtins branch from 2d10ba7 to d04fbf1 Compare October 12, 2021 10:09

BvB93 reviewed Oct 12, 2021

View reviewed changes

serge-sans-paille force-pushed the feature/provide-signature-for-some-builtins branch from d04fbf1 to 4c03b06 Compare October 12, 2021 20:22

serge-sans-paille force-pushed the feature/provide-signature-for-some-builtins branch from 4c03b06 to 564f386 Compare October 12, 2021 20:24

serge-sans-paille force-pushed the feature/provide-signature-for-some-builtins branch from 564f386 to 6851b20 Compare October 13, 2021 07:31

mattip reviewed Oct 13, 2021

View reviewed changes

serge-sans-paille force-pushed the feature/provide-signature-for-some-builtins branch 2 times, most recently from 101b541 to 1d8dad4 Compare October 13, 2021 21:10

serge-sans-paille force-pushed the feature/provide-signature-for-some-builtins branch from 1d8dad4 to 99e20c6 Compare October 15, 2021 08:17

Automate signature computation for builtin functions

25686d1

This improves the compatibility with the inspect module.

serge-sans-paille force-pushed the feature/provide-signature-for-some-builtins branch from 99e20c6 to 25686d1 Compare October 16, 2021 21:15

eric-wieser reviewed Jun 30, 2022

View reviewed changes

andfoy mentioned this pull request Sep 16, 2022

[Tracker] Fix signature mismatch with NumPy/SciPy cupy/cupy#6693

Open

	add_newdoc('numpy.core.multiarray', 'ndarray', ('ravel',
	"""
	a.ravel([order])

Search code, repositories, users, issues, pull requests...

Uh oh!

ENH: Automate signature computation for builtin functions #17818

Are you sure you want to change the base?

ENH: Automate signature computation for builtin functions #17818

Uh oh!

Conversation

serge-sans-paille commented Nov 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

serge-sans-paille commented Nov 21, 2020

Uh oh!

eric-wieser commented Nov 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

serge-sans-paille commented Nov 21, 2020

Uh oh!

eric-wieser commented Nov 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

serge-sans-paille commented Nov 23, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

serge-sans-paille commented Dec 2, 2020

Uh oh!

serge-sans-paille commented Dec 11, 2020

Uh oh!

rgommers commented Jan 2, 2021

Uh oh!

rgommers commented Jan 2, 2021

Uh oh!

serge-sans-paille commented Jan 2, 2021

Uh oh!

rgommers left a comment

Choose a reason for hiding this comment

Uh oh!

rgommers Jan 2, 2021

Choose a reason for hiding this comment

Uh oh!

rgommers Jan 2, 2021

Choose a reason for hiding this comment

Uh oh!

rgommers Jan 2, 2021

Choose a reason for hiding this comment

Uh oh!

rgommers Jan 2, 2021

Choose a reason for hiding this comment

Uh oh!

JeanKossaifi commented Oct 6, 2021

Uh oh!

serge-sans-paille commented Oct 6, 2021

Uh oh!

JeanKossaifi commented Oct 6, 2021

Uh oh!

eric-wieser commented Oct 9, 2021

Uh oh!

BvB93 Oct 12, 2021

Choose a reason for hiding this comment

Uh oh!

serge-sans-paille commented Oct 12, 2021

Uh oh!

eric-wieser commented Oct 12, 2021

Uh oh!

serge-sans-paille commented Oct 13, 2021

Uh oh!

mattip Oct 13, 2021

Choose a reason for hiding this comment

Uh oh!

serge-sans-paille commented Oct 14, 2021

Uh oh!

mattip commented Oct 14, 2021

Uh oh!

serge-sans-paille commented Oct 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mattip commented Oct 14, 2021

Uh oh!

serge-sans-paille commented Oct 14, 2021

Uh oh!

mattip commented Oct 14, 2021

serge-sans-paille commented Nov 21, 2020 •

edited

Loading

eric-wieser commented Nov 21, 2020 •

edited

Loading

eric-wieser commented Nov 22, 2020 •

edited

Loading

serge-sans-paille commented Nov 23, 2020 •

edited

Loading

serge-sans-paille commented Oct 14, 2021 •

edited

Loading

serge-sans-paille commented Oct 15, 2021 •

edited

Loading