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

Typehinting extensions proposed may not work #187

Open
GuillemBarroso opened this issue Sep 27, 2022 · 6 comments
Open

Typehinting extensions proposed may not work #187

GuillemBarroso opened this issue Sep 27, 2022 · 6 comments
Labels
documentation Improvements or additions to documentation

Comments

@GuillemBarroso
Copy link

GuillemBarroso commented Sep 27, 2022

Description of the modifications

In dev-guide, three extensions are presented in order to improve the documentation when using type hints.

However, in my case (pydpf-core and pydpf-post) the documentation was failing to build when including these extensions (following the appropriate order) in the conf.py file.

Instead, I've seen that other repos (pymapdl, pyfluent) use a different extension named "sphinx_autodoc_typehints", see their documentation. Note that the documentation states that when using this extension in conjunction with sphinx.ext.napoleon, sphinx.ext.napoleon should be loaded first, before sphinx_autodoc_typehints.

This approach seems to work for pydpf-core and pydpf-post.

Useful links and references

@GuillemBarroso GuillemBarroso added the documentation Improvements or additions to documentation label Sep 27, 2022
@greschd
Copy link
Member

greschd commented Sep 27, 2022

Thanks @GuillemBarroso, it's entirely possible this doesn't work for certain (maybe even the latest) versions of Sphinx and the extensions..

Can you link a pydpf-core or pydpf-post example where the code has type hints? AFAICR I did try that combination of extensions, but it didn't render properly (either the type hints were duplicated, or missing, I can't remember).

We do have a working example of the extensions mentioned in the dev guide in <repo I can send you privately>, I can try and check if that still works with the latest dependencies.

@GuillemBarroso
Copy link
Author

Hi @greschd,

I am waiting for approval, but the PR ansys/pydpf-post#169 seems to be handling the type hints just fine for a small example.

The typehint example can be found in plot_contour method, inside ResultData class (see result_data.py). In the PR I left a comment describing a bit what I did and why. Note that I am using sphinx_autodoc_typehints extension (see conf.py) with version 1.19.1 (see requirements_docs.txt). I had a compatibility issue with the latest 1.19.4. Also, something that I could not manage is to add the "optional" word as it was originally placed. Instead, I have added it in the parameter's description. Maybe we can find a cleaner way to specify things like this?

Please, let me know if you see any problems with this approach and let's decide what is the best way to handle type hints.

@greschd
Copy link
Member

greschd commented Sep 28, 2022

Thanks for the links @GuillemBarroso. I've tried again with the latest versions of all dependencies, and can confirm the appraoch used in pydpf-post works also for us (even with the latest sphinx-autodoc-typehints==1.19.4).

Since that approach seems to work for everyone, I'd suggest we adapt the dev guide to match it.

@MaxJPRey
Copy link
Contributor

I agree with you @greschd .
Let's try to expose this approach to bring as much consistency as possible in our projects.
Thanks @GuillemBarroso for exposing this solution.

@MaxJPRey MaxJPRey assigned MaxJPRey and GuillemBarroso and unassigned MaxJPRey Sep 28, 2022
@greschd
Copy link
Member

greschd commented Sep 28, 2022

I have run into this issue with sphinx-autodoc-typehints: tox-dev/sphinx-autodoc-typehints#123

Still investigating if this is a problem with my code, the plugin, or the warning simply needs to be ignored.

Problem with my code, the warning was right.

greschd added a commit to ansys/pyacp that referenced this issue Oct 10, 2022
See discussion in ansys/pyansys-dev-guide#187

While using `sphinx.ext.autodoc.typehints` worked for us, following
the common approach seems superior. 

Remove the `_autosummary` directory during `make(.bat) clean`  in
the documentation. This directory is auto-generated, and not
removing it can cause a rebuild of the documentation to be
incomplete.

Use `functools.wraps` for the mapping create method, to get rid of
doc build warnings.

Loosen and update the dependencies in `pyproject.toml`.
@greschd
Copy link
Member

greschd commented Feb 2, 2023

@jorgepiloto I've just noticed this is still outdated.

As far as style goes, I think we've settled on

typehints_defaults = "comma"
simplify_optional_unions = False

in ansys/pydpf-post#169

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

3 participants