DOC: add docstrings for numeric types by YannickJadoul · Pull Request #11858 · numpy/numpy

YannickJadoul · 2018-09-01T12:03:33Z

This PR adds docstrings for np.float16, np.uint8, np.uint16, np.uint32, np.uint64. Furthermore, the formatting of the char codes in the existing docstrings is unified to the same format.

Where to generate the autosummary in sphinx, adding it to the reference documentation is not completely clear, though: the current documentation on dtypes is spread out over 2 or 3 different places:

See #10106.

(Part of EuroSciPy 2018 sprints)

charris · 2018-09-01T16:27:39Z

Test failure is bogus.

jeffyancey

this looks fine to me.

numpy/core/_add_newdocs.py

seberg

Just comments, did not read all, but I think it should all be good even without going into those, just bous questions.

seberg · 2018-09-02T21:22:40Z

numpy/core/_add_newdocs.py

Wondering a bit about the Python int compatible part here. It is mostly valid for python2, and even then wrong on windows (and 32bit linux).

Yeah, this is a) not true on 32-bit windows~~, and b) doesn't set __doc__ correctly anyway~~

np.dtype('int64').__doc__ is empty, but np.int64.__doc__ shows this string.

My mistake, I was misinterpreting the results of #10106

I mainly imitated the existing documentation for all different int sizes to uint sizes. But yes, there's this aliasing going on between e.g., np.int32 is np.intc is true on my machine. So it's actually not straightforward to document this, since documenting np.intc will then also document np.int32.

seberg · 2018-09-02T21:25:22Z

numpy/core/_add_newdocs.py

c long float (or long double?). Wonder if it should warn here that it is not an IEEE quad float type. And the same text is true for float96, also... But maybe this is not the place to get into those details. Not sure how prominent this shows up also on the online docs.

Euhm, yes, that's a mistake, sorry. I'll fix this and add some extra notes on the 96-bits version, as well.

eric-wieser · 2018-09-03T23:52:18Z

~~AFAIR, setting __doc__ on the types through add_newdocs doesn't actually work - but perhaps I'm mistaken.~~

I think we need to tread carefully here - we might end up wanting different online docs to the local docs - local docs know the exact mapping of sized aliases to C types, whereas the sphinx docs shouldn't make assumptions about the user's machine.

eric-wieser · 2018-09-03T23:53:49Z

numpy/core/_add_newdocs.py

Not true - on my machine, the character code is l.

If we want to add these docs, I think we should do it to byte/short/intc/int_/longlong, not the sized aliases

YannickJadoul · 2018-09-06T16:35:40Z

we might end up wanting different online docs to the local docs - local docs know the exact mapping of sized aliases to C types, whereas the sphinx docs shouldn't make assumptions about the user's machine.

OK, this is an important point for good documentation. I will take a second stab, keeping this in mind.

To get a better picture: are these docstrings meant to also replace the list of scalar dtypes in the docs on the long run (i.e., with autosummary generated documentation) ? What level of detail is needed, and in what context should these docstrings present in the online documentation?

eric-wieser · 2018-09-06T17:07:21Z

Perhaps the correct path for now is:

Document the native unsized np.int_ types using add_new_docs, with something like

Corresponds to the C long type
Add a generated sentence like

On this platform, this type is aliased to np.int64 <docs for int64>

YannickJadoul · 2018-09-06T22:41:13Z

I'm not completely happy with how this sketch of a solution looks, but it should be a step closer to the actual correct, yet platform-flexible documentation. Is this where this PR wants to go?

eric-wieser · 2018-09-06T23:16:13Z

numpy/core/_add_newdocs.py

typo: signed

eric-wieser · 2018-09-06T23:16:55Z

numpy/core/_add_newdocs.py

Strictly it's uintptr_t

Changed size_t and ssize_t to intptr_t and uintptr_t, but the reference docs are again slightly wrong, here, then: https://docs.scipy.org/doc/numpy/user/basics.types.html

Should these be fixed as well?

eric-wieser · 2018-09-07T00:34:01Z

I like the direction this is going in - I think it will definitely be an improvement over what already exists.

I think we'll need to think a little more about how to use this with server-built sphinx builds - but for local users using help(type), I think this does exactly the right thing.

Right now none of these docstrings are in sphinx anyway, so we can punt that to a later PR.

eric-wieser · 2018-09-07T00:35:41Z

numpy/core/_add_newdocs.py

Strictly this should be double - float_ is an alias defined to be compatible with python float, although admittedly the two are always the same.

eric-wieser · 2018-09-07T00:36:45Z

numpy/core/_add_newdocs.py

I think longdouble is the canonical name.

OK, just followed the list here: https://docs.scipy.org/doc/numpy/reference/arrays.scalars.html. But indeed, longfloat seems to be an alias for longdouble.

eric-wieser · 2018-09-07T00:38:41Z

numpy/core/_add_newdocs.py

This one is an alias, not a real type

Does that mean that float96is also an alias, and either float96 or float128 is matched against longdouble?

eric-wieser · 2018-09-07T01:10:21Z

numpy/core/_add_newdocs.py

Rather than using a hack to make a local variable in a comprehension, I think this would be clearer as a generator using yield

YannickJadoul · 2018-09-07T16:41:15Z

Update: fixed minor things, but:

clongfloat is mentioned in the existing documentation, instead of clongdouble.
The reference documentation on intp mentions ssize_t instead of intptr_t.

But these can probably be fixed once these docstrings get included in the reference documentation?

Next, stackoverflow shows a few hackish way of detecting sphinx (https://stackoverflow.com/questions/20843737/check-if-sphinx-doc-called-the-script), but something feels wrong about actually accessing and using this when generating the docs?

eric-wieser · 2018-09-07T16:48:19Z

Lets leave worrying about sphinx to a later PR.

Both your points are correct - but I think the other docs are just subtly wrong, and I would prefer to aim for correctness not consistency - your most recent update looks good

eric-wieser

Can you include both the name and doc in the list of aliases? Right now I think you only add the doc.

Also, adding float_ and complex_ to the list of aliases would be handy.

Can you show the output of help(np.intc), help(np.long), and help(np.longlong) after this change?

I suspect there's some overlap here with #10151

YannickJadoul · 2018-09-07T17:06:32Z

Can you include both the name and doc in the list of aliases? Right now I think you only add the doc.

Yes, forgot that, thanks!

Also, adding float_ and complex_ to the list of aliases would be handy

Since these seem to be 'hardcoded' aliases, I'm adding these as part of the docstring itself, and not with this alias list construct.

eric-wieser · 2018-09-07T17:21:54Z

I'm adding these as part of the docstring itself

Fine by me.

YannickJadoul · 2018-09-07T17:29:06Z

Can you show the output of help(np.intc), help(np.long), and help(np.longlong) after this change?

>>> print(np.intc.__doc__)
Signed integer type, compatible with C ``int``.
    Character code: ``'i'``.
>>> print(np.long.__doc__)
int(x=0) -> integer
int(x, base=10) -> integer

Convert a number or string to an integer, or return 0 if no arguments
are given.  If x is a number, return x.__int__().  For floating point
numbers, this truncates towards zero.

If x is not a number or if base is given, then x must be a string,
bytes, or bytearray instance representing an integer literal in the
given base.  The literal can be preceded by '+' or '-' and be surrounded
by whitespace.  The base defaults to 10.  Valid bases are 0 and 2-36.
Base 0 means to interpret the base from the string as an integer literal.
>>> int('0b100', base=0)
4
>>> print(np.longlong.__doc__)
Signed integer type, compatible with C ``long long``. 
    Character code: ``'q'``.

I'll have a closer look at that other PR later.

eric-wieser · 2018-09-07T17:34:24Z

I'd specifically like to see the output of help. Also I made a typo, and said np.long not np.int_.

eric-wieser · 2018-09-07T17:37:50Z

numpy/core/_add_newdocs.py

I think this should be AttributeError.

Stolen from add_newdoc in core/function_base.py, where it also says Exception, but I've adapted it.

YannickJadoul · 2018-09-09T15:44:37Z

I'd specifically like to see the output of help.

Sorry, here you go:

help(np.intc)

Help on class int32 in module numpy:

class int32(signedinteger)
 |  Signed integer type, compatible with C ``int``.
 |  Character code: ``'i'``.
 |  
 |  Method resolution order:
 |      int32
 |      signedinteger
 |      integer
 |      number
 |      generic
 |      builtins.object
 |  
 |  Methods defined here:
 |  
 |  __abs__(self, /)
 |      abs(self)
 |  
 |  __add__(self, value, /)
 |      Return self+value.
 |  
 |  __and__(self, value, /)
 |      Return self&value.
[...]
 |  astype(...)
 |      Not implemented (virtual attribute)
 |      
 |      Class generic exists solely to derive numpy scalars from, and possesses,
 |      albeit unimplemented, all the attributes of the ndarray class
 |      so as to provide a uniform API.
 |      
 |      See Also
 |      --------
 |      The corresponding attribute of the derived class of interest.
 |  
[...]

`help(np.int_)

Help on class int64 in module numpy:

class int64(signedinteger)
 |  Signed integer type, compatible with Python `int` anc C ``long``.
 |  Character code: ``'l'``.
 |  
 |  Method resolution order:
 |      int64
 |      signedinteger
 |      integer
 |      number
 |      generic
 |      builtins.object
[...]

help(np.longlong)

Help on class int64 in module numpy:

class int64(signedinteger)
 |  Signed integer type, compatible with C ``long long``. 
 |  Character code: ``'q'``.
 |  
 |  Method resolution order:
 |      int64
 |      signedinteger
 |      integer
 |      number
 |      generic
 |      builtins.object
[...]

eric-wieser

Can't comment inline on mobile: Line 8017 should not be passing a default to getattr - the attribute will always exist for canonical names

eric-wieser · 2018-09-16T04:28:54Z

numpy/core/_add_newdocs.py

+def add_newdoc_for_numeric_type(obj, fixed_aliases, possible_aliases, doc):
+    o = getattr(_numerictypes, obj, None)
+    if o is None:
+        return


From my earlier comment - change this to o = getattr(_numerictypes, obj), since the object is guaranteed to exist.

eric-wieser · 2018-09-16T04:29:45Z

numpy/core/_add_newdocs.py

+        for (alias, doc) in aliases:
+            alias_type = getattr(_numerictypes, alias, None)
+            if alias_type is not None:
+                yield (alias_type, alias, doc)


Strictly speaking this would be better as:

try: alias_type = getattr(_numerictypes, alias) except AttributeError: pass else: yield (alias_type, alias, doc)

As that doesn't silence bugs if an alias ends up somehow being set to None

eric-wieser · 2018-09-16T04:31:30Z

numpy/core/_add_newdocs.py


-add_newdoc('numpy.core.numerictypes', 'object_',
-    """Any Python object.  Character code: 'O'.""")
+add_newdoc_for_numeric_type('object_', [], [],


Not sure I'd consider this numeric. Probably better either to rename the function to *_scalar_type, or leave the non-numeric types as they were. I realize the module is called numerictypes, but we're stuck with that for now.

eric-wieser

Made the changes I suggested below myself. Feel free to tweak if you want, but I think this is ready to go in

YannickJadoul · 2018-09-16T11:04:24Z

Made the changes I suggested below myself. Feel free to tweak if you want, but I think this is ready to go in

I've still implemented the one small change in numeric_type_aliases you suggested.

YannickJadoul · 2018-09-16T11:12:43Z

Oh, and almost forgot, but I yesterday noticed that this actually doesn't work for the complex types. These types already have a docstring set in the C code, and add_newdocs will not overwrite the existing one (but does so silently):

>>> help(np.cdouble)
Help on class complex128 in module numpy:

class complex128(complexfloating, builtins.complex)
 |  Composed of two 64 bit floats

I guess I've missed this since been focusing on the signed and unsigned integers, since they had these C type equivalence issues.

The line with this docstring is here: https://github.com/numpy/numpy/blob/master/numpy/core/src/multiarray/scalartypes.c.src#L3773. My instinctive reaction would be to make this consistent and remove the docstring from the C code, but then I don't know the reason why these docstrings are there?

eric-wieser · 2018-09-16T22:50:51Z

My instinctive reaction would be to make this consistent and remove the docstring from the C code

Seems sensible to me. Even if there is a reason for them to be there, it would apply to all of the docstrings anyway, not just that one,

eric-wieser · 2018-09-16T22:52:03Z

numpy/core/_add_newdocs.py

+    ('complex128', 'Complex number type composed of 2 64-bit-precision floating-point numbers'),
+    ('complex192', 'Complex number type composed of 2 96-bit extended-precision floating-point numbers'),
+    ('complex256', 'Complex number type composed of 2 128-bit extended-precision floating-point numbers'),
+    ])


Is there any real value to having these four separate lists of aliases, rather than building one big list to perform the lookup in?

Euhm, yeah, the idea was not not loop over irrelevant aliases (while. But that does seem to come at the cost of being slightly more error-prone. I'm guessing you prefer the side of the trade-off that's less error-prone?

Yeah, especially since collectivey we've proven that that type of error is easy to make and hard to spot. Thanks!

One less argument to add_newdoc_for_scalar_type as well, as bonus.

…ultiarray/scalartypes.c.src, as they are now set in numpy/core/_add_newdocs.py

…ing 'np.' to fixed aliases

eric-wieser · 2018-09-17T08:12:31Z

numpy/core/_add_newdocs.py

+            try:
+                alias_type = getattr(_numerictypes, alias)
+            except AttributeError:
+                pass


Could do with a comment here like "the set of aliases that actually exist varies between platforms" or "this alias is not present on this platform" or something

eric-wieser

If you want more visibility for this, a release note under "improvements" mentioning that help(np.intp) or similar now shows a list of common type aliases would seem pretty sensible

YannickJadoul · 2018-09-17T10:34:40Z

Almost done! :-)

Running the code from #10106 again, there are a few more undocumented numeric types that seem to be related, part of the type hierarchy of scalar types:

<class 'numpy.complexfloating'> (np.complexfloating)
<class 'numpy.flexible'> (np.flexible)
<class 'numpy.floating'> (np.floating)
<class 'numpy.inexact'> (np.inexact)
<class 'numpy.integer'> (np.integer)
<class 'numpy.number'> (np.number)
<class 'numpy.signedinteger'> (np.signedinteger)
<class 'numpy.unsignedinteger'> (np.unsignedinteger)

Futhermore, numpy.void seems like it could still be a couple of lines included in this PR as well, just after object ?

mattip · 2018-09-17T11:35:26Z

Perhaps let this go in as-is, and add the rest in another PR?

eric-wieser · 2018-09-17T15:17:55Z

I'm with @mattip on this one - documenting the abstract types seems like a separate task to documenting the concrete ones.

eric-wieser · 2018-09-17T15:23:54Z

Just tweaked the release notes - I plan to squash and merge in a day or two, in case anyone else decides to weigh in.

charris · 2018-09-18T19:49:14Z

Thanks @YannickJadoul. And thanks to Eric for helping get this knocked into shape.

YannickJadoul · 2018-09-18T20:42:40Z

Thanks indeed, @eric-wieser, for the guidance and remarks!

YannickJadoul mentioned this pull request Sep 1, 2018

DOC: add docstring for np.float16 (see #10106) #11854

Closed

jeffyancey approved these changes Sep 2, 2018

View reviewed changes

numpy/core/_add_newdocs.py Outdated Show resolved Hide resolved

YannickJadoul force-pushed the np.core.numerictypes-doc branch from 97ac6b4 to 5c6c027 Compare September 2, 2018 20:20

seberg reviewed Sep 2, 2018

View reviewed changes

mattip mentioned this pull request Sep 3, 2018

DOC: Make clear the connection between numpy types and C types #11837

Merged

eric-wieser reviewed Sep 3, 2018

View reviewed changes

YannickJadoul force-pushed the np.core.numerictypes-doc branch from 9b9b2f2 to 05c601c Compare September 6, 2018 21:45

eric-wieser reviewed Sep 6, 2018

View reviewed changes

numpy/core/_add_newdocs.py Outdated

Copy link

Member

eric-wieser Sep 6, 2018

Choose a reason for hiding this comment

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

typo: signed

eric-wieser reviewed Sep 6, 2018

View reviewed changes

eric-wieser reviewed Sep 7, 2018

View reviewed changes

charris added 04 - Documentation component: numpy._core labels Sep 9, 2018

eric-wieser reviewed Sep 15, 2018

View reviewed changes

eric-wieser reviewed Sep 16, 2018

View reviewed changes

MAINT: Remove dead code, rename function

9731928

eric-wieser approved these changes Sep 16, 2018

View reviewed changes

DOC: Cleaning up numeric_type_aliases in core/_add_newdocs.py

71a383c

DOC: Fixing dynamic aliases in docstrings for complex scalar types

722c30e

eric-wieser reviewed Sep 16, 2018

View reviewed changes

YannickJadoul added 2 commits September 17, 2018 09:55

DOC: Removing docstrings for complex scalar types in numpy/core/src/m…

f073811

…ultiarray/scalartypes.c.src, as they are now set in numpy/core/_add_newdocs.py

DOC: Merging lists of different dynamic aliases into one, and prepend…

857cd9a

…ing 'np.' to fixed aliases

eric-wieser reviewed Sep 17, 2018

View reviewed changes

eric-wieser approved these changes Sep 17, 2018

View reviewed changes

eric-wieser added this to the 1.16.0 release milestone Sep 17, 2018

DOC: Adding scalar type docstring improvements to release notes

dff5de6

YannickJadoul force-pushed the np.core.numerictypes-doc branch from c47a0a7 to dff5de6 Compare September 17, 2018 10:28

DOC: Tweak release notes

1561b9c

eric-wieser mentioned this pull request Sep 17, 2018

DOC: Add missing documentation for top-level functions #10106

Closed

32 tasks

charris merged commit 9741ce2 into numpy:master Sep 18, 2018

YannickJadoul deleted the np.core.numerictypes-doc branch September 18, 2018 20:42

YannickJadoul mentioned this pull request Jul 15, 2020

Did you know that py::buffer_info::format has a different meaning on Windows? pybind/pybind11#1908

Open

Uh oh!

Conversation

YannickJadoul commented Sep 1, 2018

Uh oh!

charris commented Sep 1, 2018

Uh oh!

jeffyancey left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

seberg left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eric-wieser Sep 3, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

eric-wieser commented Sep 3, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

YannickJadoul commented Sep 6, 2018

Uh oh!

eric-wieser commented Sep 6, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

YannickJadoul commented Sep 6, 2018

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!

eric-wieser commented Sep 7, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Choose a reason for hiding this comment

Uh oh!

YannickJadoul commented Sep 7, 2018

Uh oh!

eric-wieser commented Sep 7, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eric-wieser left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

YannickJadoul commented Sep 7, 2018

Uh oh!

eric-wieser commented Sep 7, 2018

Uh oh!

YannickJadoul commented Sep 7, 2018

Uh oh!

eric-wieser commented Sep 7, 2018

eric-wieser Sep 3, 2018 •

edited

Loading

eric-wieser commented Sep 3, 2018 •

edited

Loading

eric-wieser commented Sep 6, 2018 •

edited

Loading

eric-wieser commented Sep 7, 2018 •

edited

Loading

eric-wieser commented Sep 7, 2018 •

edited

Loading

eric-wieser left a comment •

edited

Loading

eric-wieser Sep 16, 2018 •

edited

Loading

eric-wieser Sep 16, 2018 •

edited

Loading

eric-wieser Sep 16, 2018 •

edited

Loading

eric-wieser Sep 17, 2018 •

edited

Loading