ENH: Show full PEP 457 argument lists for ufuncs #9026

eric-wieser · 2017-04-29T20:44:20Z

Describes the full argument lists of ufunc.__call__ using PEP 457 syntax.

Before:

add(x1, x2, [out])
frexp(x, [out1, out2,])

After:

add(x1, x2, /, out=None, *, where=True, casting='same_kind', order='K', dtype=None, subok=True, signature, extobj)
frexp(x[, out1, out2], /[, out=(None, None)], *, where=True, casting='same_kind', order='K', dtype=None, subok=True[, signature, extobj])

In future, this could be hooked up to __text_signature__ (#8734) to make inspect.signature work on ufuncs (right now, this requires monkey-patching inspect).

~~This needs to be checked for compatibility against sphinx/numpydoc, which might choke on this signature format~~ Done and working!

eric-wieser · 2017-04-29T20:46:52Z

If we want to make inspect.signature work, we'd:

Add a trivial __text_signature__ attribute that contains what we calculate here
Monkey-patch as inspect._NonUserDefinedCallables += (np.ufunc,)

It's worth noting that inspect.signature does not support the square bracket syntax suggested by PEP457

mhvk · 2017-05-01T17:25:50Z

I now looked in a bit more detail, and this looks all good. It might make sense to have it in 1.13 still, but perhaps @charris should decide on that.

charris · 2017-05-01T17:41:30Z

@mhvk If you think it is good, put it in. The 1.13.0 milestone is for things I think need to be in 1.13, and some of those I'll probably push off again to 1.14.

eric-wieser · 2017-05-01T17:46:47Z

Note that if we do put this in, the arguments still wont be described on the individual ufunc pages - so somehow, we should probably link back to the ufunc docs

mhvk · 2017-05-01T18:06:12Z

@eric-wieser - you mean that these changes are visible in, say, ipython, but won't be seen by sphinx?

eric-wieser · 2017-05-01T18:17:59Z

I mean that the top of the sphinx page will list all the arguments, but nowhere on the page will a description of the arguments be given.

Having said that, I haven't actually tested that sphinx supports this syntax...

IPython seems to show the new longer argument lists, which was my main aim here

eric-wieser · 2017-05-01T18:24:29Z

Having said that, I haven't actually tested that sphinx supports this syntax...

And havehad difficulty in the past on windows getting numpy to build docs from the source, rather than the installed version

mhvk · 2017-05-01T18:25:11Z

Ah, yes, so the general description has to be adapted too. In that case, my suggestion would be to merge this with 1.14 as a milestone, and raise a new issue to also adjust the overall documentation.

eric-wieser · 2017-05-01T18:26:57Z

In that case, my suggestion would be to merge this with 1.14 as a milestone

Huh? Do you mean wait until 1.13 is branched, and then merge? Or set the milestone to 1.14 yet merge it into 1.13?

eric-wieser · 2017-05-01T23:27:13Z

Ah, yes, so the general description has to be adapted too.

Done. All this needs is someone with a working doc build setup to check that the pages for np.add and np.fr 8000 exp look right under sphinx.

eric-wieser · 2017-05-01T23:28:41Z

numpy/core/code_generators/ufunc_docstrings.py

+        Alternate array object(s) in which to put the result; if provided, it
+        must have a same shape that the inputs broadcast to.
+    where : array_like, optional
+        Values of True indicate to calculate the ufunc at that position, values


As a bonus, this now means that where is documented everywhere.

We could add the others here, but the explanations are more complicated, and they'd risk diverging from ufuncs.kwargs

eric-wieser · 2017-05-01T23:29:34Z

numpy/core/code_generators/ufunc_docstrings.py

 def add_newdoc(place, name, doc):
+    doc = textwrap.dedent(doc).strip()


dedenting here makes substituting $PARAMS do the right thing

mhvk · 2017-05-02T00:55:27Z

numpy/core/code_generators/ufunc_docstrings.py

+    where : array_like, optional
+        Values of True indicate to calculate the ufunc at that position, values
+        of False indicate to leave the value in the output alone.
+


To make it clearer that there are other arguments, I'd put this as part of the parameter list:

**kwargs For other keyword-only ...

mhvk · 2017-05-02T01:21:54Z

For reasons slightly beyond me, I have similar problems with the sphinx build using the system installation of numpy despite being in a virtual environment in which I have installed the PR. So, I cannot immediate check that things work there.

Inside ipython, things look good, except I think one might as well adjust the class docstring as well...

eric-wieser · 2017-05-02T10:32:21Z

I think one might as well adjust the class docstring as well...

Done

eric-wieser · 2017-05-03T16:21:22Z

8000

For reasons slightly beyond me, I have similar problems with the sphinx build using the system installation of numpy despite being in a virtual environment in which I have installed the PR.

The solution I found is to use runtests.py --shell, and then building the docs from there.

The signature works just fine, but the link doesn't work:

Oddly, sphinx seems to move the square brackets around, but it doesn't seem to matter

pv · 2017-05-03T16:39:55Z

I think numpydoc doesn't like the link in the parameter list, as there was no allowance in the spec for stuff after the parameter list. . The extra explanation can probably be put in a Notes section or so.

eric-wieser · 2017-05-03T16:40:48Z

@pv: I'm running into #9046 here, which means those screenshots do not match my source. There's also some caching within sphinx that is making this hard to iterate on

eric-wieser · 2017-05-03T16:43:13Z

Ok, after doing a truly clean build, this now works:

eric-wieser · 2017-05-07T20:52:56Z

Arguments for putting this in 1.13: We're now allowing people to intercept all the ufunc arguments in __array_ufunc__, so it'd be handy to give people a reminder of what they are, without having to resort to the online docs

mhvk · 2017-05-07T21:07:08Z

I think you ended up making the full change, right? If so, I think 1.13 is indeed appropriate.

eric-wieser · 2017-05-07T21:08:03Z

@mhvk: Yep, this is complete and tested. although the later of this and #9063 to be merged will require changes

mhvk

Had another look and found a few small things to change, but overall, this looks great!

mhvk · 2017-05-07T21:11:02Z

numpy/add_newdocs.py

@@ -5421,40 +5421,19 @@ def luf(lamdaexpr, *args, **kwargs):
    """
    Functions that operate element by element on whole arrays.

-    To see the documentation for a specific ufunc, use np.info().  For
-    example, np.info(np.sin).  Because ufuncs are written in C
+    To see the documentation for a specific ufunc, use `np.info`.  For


I think this needs to be double ticks to, or does it link correctly? (I ask since in astropy this would require ~numpy.info; if the link does work, maybe pre-pend :func:)

Forgot to check this. I think you're probably right there.

This page never gets generated anyway in the html docs, so this is kinda irrelevant

Switched to info for simplicity, in case this does get linked in future

mhvk · 2017-05-07T21:15:13Z

numpy/core/code_generators/ufunc_docstrings.py

 def add_newdoc(place, name, doc):
+    doc = textwrap.dedent(doc).strip()
+    doc = doc.replace('$PARAMS', _params_text)


Bit odd to have this shell-like contruct. Why not just call it {PARAMS} in the strings, and here do

doc = doc.format(PARAMS=_params_text)

Latex in the docstrings was my concern here

OK, that seems fair enough. Just leave as is, then.

mhvk · 2017-05-07T21:19:05Z

numpy/core/code_generators/ufunc_docstrings.py

@@ -3067,6 +3083,7 @@ def add_newdoc(place, name, doc):
    ----------
    x1 : array_like


Might as well change this to x

Yep, since right now it doesn't actually match the injected signature

mhvk · 2017-05-07T21:21:05Z

I think after #9063 makes most sense, but I'm changing the milestone so this won't be forgotten!

This was covered by very few ufuncs, and is probably better to leave out.

eric-wieser · 2017-05-08T00:29:37Z

@mhvk: Rebased to include the modifications to the divmod docstring from #9063

charris · 2017-05-08T17:54:02Z

numpy/add_newdocs.py

-    out : array_like
-        An array to store the output. Must be the same shape as the
-        output would have.
+    *x : array_like


Each of the * need escaping like so \\* for sphinx. If you make it a raw string you can use a single \.

This file never actually reaches sphinx anyway, but I was under the impression that this worked correctly due to the transformation that the Parameters section undergoes

I've had to fix other uses before as the docs didn't render correctly.

Well, this doesn't even get rendered, so no danger of it getting rendered incorrectly!

Since the only place this is ever seen is in the terminal, I'd argue it's better without backslashes.

I'd also argue that to keep the terminal tidy, the fix should go in the numpy sphinx extension, not here

I agree that having numpydoc escape the *'s when generating the sphinx input would be nice, but it doesn't at this time. If this never showed up in the documentation, that would not be a problem, but I assume you would like it to eventually show up.

but I assume you would like it to eventually show up.

Not really - AFAICT, the sole purposed of this page is to redirect the user to np.info or the ufunc docs. if ufunc.__doc__ is made visible in the web docs, then IMO, it should be the contents of ufuncs.rst.

This docstring aside - I was pretty sure that **kwargs was parsed correctly in the other docstring modified by this changeset - does the issue only exist for single asterisks?

charris · 2017-05-08T17:58:26Z

numpy/add_newdocs.py

+    where : array_like, optional
+        Values of True indicate to calculate the ufunc at that position, values
+        of False indicate to leave the value in the output alone.
+    **kwargs :


Don't know if this needs something after the : or not, but might be safer to put a placeholder there.

That : should probably be removed, but again - no HTML docs are generated from this page

Irrelevant given that there are documents generated, but yes, with the : removed it works fine.

Fixed, anyway

The distinction between unary and binary is not too helpful, and ignores the fact that trinary ufuncs can exist. Also add a link to the ufunc docs

charris · 2017-05-09T02:47:31Z

This docstring aside - I was pretty sure that **kwargs was parsed correctly in the other docstring modified by this changeset

Not sure. The * gets translated as emphasis IIRC. Grepping the source, the two versions I see are \\*\\*kwargs or just plain kwargs without the asterisks. The latter seems like a viable workaround as long as it comes with an optional arguments explanation or some such. The same variations hold for args. I suspect a fix for numpydocs would not be too difficult if one finds where the parameters are parsed out.

eric-wieser · 2017-05-09T12:25:21Z

Ok, see the screenshot in the above comment - the ** is definitely handled correctly.

mhvk · 2017-05-09T12:31:51Z

**kwargs works fine -- see also http://docs.astropy.org/en/latest/api/astropy.coordinates.SkyCoord.html#astropy.coordinates.SkyCoord (astropy uses the numpy setup with some additions)

eric-wieser · 2017-05-09T12:34:47Z

@mhvk: Do you have an example of *args working too, to save me the trouble of a local build?

eric-wieser · 2017-05-09T12:48:49Z

@charris: tested with unescaped * in the arglist, and it works just fine (sphinx 1.5.6):

Is this then good to merge?

charris · 2017-05-09T14:12:46Z

Looks like the escape problem has been fixed, good to know. Now we can clean up the other cases ;)

charris · 2017-05-09T14:13:09Z

Thanks Eric.

eric-wieser added 04 - Documentation 01 - Enhancement component: numpy._core labels Apr 29, 2017

eric-wieser changed the title ~~ENH: Show full argument lists for ufuncs~~ ENH: Show full PEP 457 argument lists for ufuncs Apr 29, 2017

eric-wieser force-pushed the ufunc_docstrings branch 2 times, most recently from 44c1506 to a01b003 Compare May 1, 2017 09:58

eric-wieser force-pushed the ufunc_docstrings branch from a01b003 to bd3e35a Compare May 1, 2017 23:25

eric-wieser commented May 1, 2017

View reviewed changes

eric-wieser force-pushed the ufunc_docstrings branch from 267b035 to 18f226d Compare May 1, 2017 23:56

mhvk reviewed May 2, 2017

View reviewed changes

eric-wieser force-pushed the ufunc_docstrings branch 2 times, most recently from d21b48c to d0e2800 Compare May 2, 2017 10:31

charris added this to the 1.14.0 release milestone May 5, 2017

eric-wieser mentioned this pull request May 6, 2017

ENH: add np.divmod ufunc #9063

Merged

3 tasks

mhvk reviewed May 7, 2017

View reviewed changes

mhvk modified the milestones: 1.13.0 release, 1.14.0 release May 7, 2017

eric-wieser added 3 commits May 8, 2017 01:26

DOC: Show full argument lists for ufuncs

476ce74

DOC: Refer to main ufunc docs for kwargs

ba6c1af

DOC: Remove explanation of exception due to out

bcc77f9

This was covered by very few ufuncs, and is probably better to leave out.

eric-wieser force-pushed the ufunc_docstrings branch from ae1cfe5 to 1f3f57a Compare May 8, 2017 00:28

charris reviewed May 8, 2017

View reviewed changes

eric-wieser added 2 commits May 8, 2017 19:42

DOC: Simplify the output of help(ufunc)

0025c86

The distinction between unary and binary is not too helpful, and ignores the fact that trinary ufuncs can exist. Also add a link to the ufunc docs

DOC: Correct parameter names for spacing and invert

2e32026

eric-wieser force-pushed the ufunc_docstrings branch from 1f3f57a to 2e32026 Compare May 8, 2017 18:47

charris merged commit beb4ae2 into numpy:master May 9, 2017

This was referenced May 9, 2017

ENH: Allow Randint to Broadcast Arguments #6938

Closed

BUG: don't silence warnings in ufunc.reduce #9020

Merged

eric-wieser mentioned this pull request May 9, 2017

Document ufunc kwargs in docstrings #7190

Closed

eric-wieser mentioned this pull request Jun 27, 2017

DOC: Use x1 and x2 in the heaviside docstring. #9320

Merged

		def add_newdoc(place, name, doc):
		doc = textwrap.dedent(doc).strip()

		@@ -3067,6 +3083,7 @@ def add_newdoc(place, name, doc):
		----------
		x1 : array_like

Uh oh!

ENH: Show full PEP 457 argument lists for ufuncs #9026

ENH: Show full PEP 457 argument lists for ufuncs #9026

Uh oh!

Conversation

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment