Skip to content
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

DOC: add autosummary API reference for DType clases. #25656

Merged
merged 2 commits into from
Jan 23, 2024

Conversation

ngoldbaum
Copy link
Member

This was supposed to be included in #25507 and was mistakenly left out, introducing some reference warnings.

The net effect of this PR should be to add a new docs page available at reference/generated/numpy.dtypes.html, linked to by another page at reference/routines.dtypes.html.

Note that this introduces new reference warnings, due to an issue with how the descriptor and dtypemeta classes are implemented in C, as explained here:

One wrinkle I noticed looking at the docs build, this introduces some new reference warnings that I can't easily fix:

/Users/goldbaum/Documents/numpy/build-install/usr/local/lib/python3.11/site-packages/numpy/dtypes.py:docstring of numpy.dtypes.BytesDType:87: 
 WARNING: autosummary: stub file not found 'numpy.dtypes.BytesDType.type'. Check your autosummary_generate setting.

And similar warnings for all of the other DType classes. I think that's happening is that both the dtype metaclass and dtype class share a python attribute named type, and the way we've implemented the python inheritance for this in C (perhaps also somehow combined with the use of T_OBJECT to define the attribute) is confusing sphinx. If I change dtypemeta to use a scalar_type python-level name, that fixes the sphinx issue, but that seems like a more invasive risky change that I want to take on in this PR.

I would personally rather have these spurious reference warnings in the build and also be able to cross-reference the DType classes elsewhere in the docs, but please let me know if we'd rather not introduce the new reference warnings and we need a more permanent fix.

[skip cirrus] [skip actions] [skip azp]
@seberg
Copy link
Member

seberg commented Jan 22, 2024

Not sure what one can do. Is there a way to manually override one attribute in the docs?
I don't think this one is so bad (but others may care, and I am happy with that). What is actually worse maybe is: https://numpy.org/doc/stable/reference/generated/numpy.dtype.type.html

DTypeMeta.type seems like it should work really. Feels a bit much to use different names just due to this, but this isn't nice.


Otherwise, this is good to add, and I am 👍 to just do it, adding seems unrelated to the problems here.

Copy link
Contributor

@rossbar rossbar left a comment

Choose a reason for hiding this comment

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

I vote to get this in despite reflink warnings. I can't think of a quick solution, but IMO it shouldn't be a blocker to getting this in!

doc/source/reference/routines.dtypes.rst Outdated Show resolved Hide resolved
Co-authored-by: Ross Barnowski <rossbar@berkeley.edu>
@ngoldbaum
Copy link
Member Author

Pulling this in, thanks for taking a look!

@ngoldbaum ngoldbaum merged commit a7681f6 into numpy:main Jan 23, 2024
63 checks passed
@rgommers rgommers added this to the 2.0.0 release milestone Jan 24, 2024
@rgommers
Copy link
Member

Thanks @ngoldbaum for addressing this. I'll note that I will undo the changes here, because an empty page with a single entry isn't quite right (I just got rid of other such pages):

image

The warnings can probably get rid of with a :no-special-members: or other such magic directive tweak, I'll give it a go.

@rgommers
Copy link
Member

I've tried basically every possible way of doing this. What came closest was autodoc-skip-member, like so in doc/source/conf.py:

def skip_member(app, what, name, obj, skip, options):
    if name == 'type':
        return True
    return False


def setup(app):
    # Fix for warnings for dtype.type (see gh-25656)
    app.connect('autodoc-skip-member', skip_member)

That can actually intercept the search for the type method/attribute, but returning True for a class member still doesn't make Sphinx happy (looks like a bug, it does work for skipping functions or classes).

This is a problem that we need to fix. These are the only warnings in the doc build with the default settings of spin docs, and it's a wall of red that's going to continue to be annoying and waste people's time trying to figure out what to do about.

I think we just keep the nice table in the docs, but revert the auto-generated docs for every class. They don't have that much value, and are a problem in themselves - due to the large amount of repetition, it increases the doc build time by over a minute. And it's all effectively boilerplate - if the docs just say "inherits all the attributes and methods from numpy.dtype" (with numpy.dtype a link), that should be good enough. Creating about 32 classes x 23 attributes/methods = 736 separate extra doc pages doesn't add that much. As long as they can be linked to, it's better for them to look like https://numpy.org/devdocs/reference/arrays.scalars.html#numpy.int8 & co.

@ngoldbaum
Copy link
Member Author

That sounds totally reasonable, as long as we can still cross-reference them and they show up in a docs search. I can take a crack at doing this tomorrow if you'd like.

@rgommers
Copy link
Member

I can take a crack at doing this tomorrow if you'd like.

That would be helpful, thanks 🙏🏼. Hopefully copying the :autoclass: approach as for int8 won't be too troublesome.

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.

None yet

4 participants