ansys / pyansys-dev-guide

PyAnsys Project Developer's Guide
http://dev.docs.pyansys.com
MIT License
33 stars 7 forks source link

Typehinting extensions proposed may not work #187

Open GuillemBarroso opened 1 year ago

GuillemBarroso commented 1 year ago

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

greschd commented 1 year ago

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 commented 1 year ago

Hi @greschd,

I am waiting for approval, but the PR pyansys/dpf-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 commented 1 year ago

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 commented 1 year ago

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.

greschd commented 1 year ago

I have run into this issue with sphinx-autodoc-typehints: https://github.com/tox-dev/sphinx-autodoc-typehints/issues/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 commented 1 year ago

@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 https://github.com/pyansys/pydpf-post/pull/169