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

type annotations make docs confusing #3178

Closed
dcherian opened this issue Aug 2, 2019 · 17 comments · Fixed by #3179 or #4070
Closed

type annotations make docs confusing #3178

dcherian opened this issue Aug 2, 2019 · 17 comments · Fixed by #3179 or #4070

Comments

@dcherian
Copy link
Contributor

dcherian commented Aug 2, 2019

The annotations make this signature basically unreadable to anyone not familiar with them.

image

Is there a way to hide them in the documentation? Or at least change formatting so it's clearer what the function arguments are?

@DocOtak
Copy link
Contributor

DocOtak commented Aug 2, 2019

Perhaps the sphinx-autodoc-typehints extension?

The docs suggest it will remove the types from the method signatures and put them in the in :param: parts. I haven't used or tested myself.

@dcherian
Copy link
Contributor Author

dcherian commented Aug 2, 2019

Thanks @DocOtak. Are you interested in trying it out?

@DocOtak
Copy link
Contributor

DocOtak commented Aug 2, 2019

@dcherian sure, I'll try it right now with the xarray docs

@shoyer
Copy link
Member

shoyer commented Aug 2, 2019

If we can't get that sphinx extension to work (I'm not sure if it will handle NumPy style docstrings), another option might to be define type aliases for the longer types, e.g., data_vars: DataVars. That would at least cut down on the noise.

@DocOtak
Copy link
Contributor

DocOtak commented Aug 2, 2019

So the plugin seems to "just works" in that it remove these data type annotation, it doesn't seem to put them anywhere. I can probably put the docs I built somewhere if you all want to look at them. Here is a screen shot of the "Dataset" class, first one is just the extension, second screenshot also has the napoleon extension enabled. Main difference is how the "parameters" appear.
sphinx_dtype
sphinx_dtype_napoleon

@dcherian
Copy link
Contributor Author

dcherian commented Aug 2, 2019

EDIT: ignore, I see you've tried napoleon

@dcherian
Copy link
Contributor Author

dcherian commented Aug 2, 2019

Have you enabled napoleon_use_param? (https://stackoverflow.com/questions/49540656/how-to-automatically-add-parameter-types-in-sphinx-documentation)

Add 'sphinx_autodoc_typehints' to the extensions list in conf.py after 'sphinx.ext.napoleon', and make sure you also add napoleon_use_param = True to conf.py.

@DocOtak
Copy link
Contributor

DocOtak commented Aug 2, 2019

So I think why it isn't putting the types anywhere in the docs is because they already exist (at least for this Dataset __init__ that we are looking at).

The relevant part of the code in the extension appears to be this https://github.com/agronholm/sphinx-autodoc-typehints/blob/master/sphinx_autodoc_typehints.py#L333:L338

It's looking for :param name: and I think things with types are already :param type name: with napoleon enabled, so it doesn't find anything to replace. Without napoleon enabled, the :param name: fields are not present since it is "raw" numpy doc style.

@DocOtak
Copy link
Contributor

DocOtak commented Aug 2, 2019

Suspicions confirmed. I removed the type parts in the docstrings. The attached is the result which I think is way less readable:
sphinx_dtype_napoleon2

@shoyer
Copy link
Member

shoyer commented Aug 2, 2019

I would be OK with stripping type hints entirely. Our long type annotations are mostly useful for tooling. The short annotations from our numpy-style docstrings are definitely more readable for humans.

@dcherian
Copy link
Contributor Author

dcherian commented Aug 2, 2019

+1 I like the style with Napoleon enabled. It even fixes #3056.

@shoyer
Copy link
Member

shoyer commented Aug 2, 2019

Yes, this is great! Please send a PR when you're ready :)

@DocOtak
Copy link
Contributor

DocOtak commented Aug 2, 2019

So I made a PR for just removing the type annotations, turns out it is built in to autodoc. Enabling napoleon seems to be less "clean". While it doesn't actually conflict with numpydoc it does appear to "compete" with it. It only really seemed to affect the autowrapped ufunc documentation. I'm going to do a separate "enable napoleon" PR

@DocOtak
Copy link
Contributor

DocOtak commented Aug 2, 2019

See #3180 for the napoleon enabling PR.

@dcherian
Copy link
Contributor Author

Looks like this isn't fully fixed

image

@dcherian dcherian reopened this Nov 19, 2019
@DocOtak
Copy link
Contributor

DocOtak commented Nov 19, 2019

Any recollection as to if these ever worked as expected? Looks like between landing this change and doing the 0.14 release, the sphinx version bumped from 2.1.2 to 2.2.0 which included some changes to autodoc... This PR might be of interest sphinx-doc/sphinx#6592 but it is not immediately obvious to me how/if this could have broken things.

@crusaderky
Copy link
Contributor

I tried using sphinx-autodoc-typehints in my own project but I'm getting a gazillion of errors.
TypeVars don't work, Optional doesn't work, Literal doesn't work, intersphinx doesn't work (https://github.com/agronholm/sphinx-autodoc-typehints/issues/119), possibly more (the error log is just too long). :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants