ansys / pydpf-post

Data Processing Framework - Post Processing Module
https://post.docs.pyansys.com
MIT License
43 stars 10 forks source link

Improve TypeHints #164

Closed GuillemBarroso closed 2 years ago

GuillemBarroso commented 2 years ago

Resolves #160.

@PProfizi, note that the extensions are different in conf.py files from dpf-post and dpf-core. I was tempted to add all extensions from dpf-core to dpf-post, but in dpf-post we have 2 that are missing in dpf-core (notfound.extension and sphinx.ext.autosummary). I do not know if this is worth an issue and try to unify the extensions criterion. Thoughts?

codecov[bot] commented 2 years ago

Codecov Report

Merging #164 (3494ceb) into master (2ffde44) will not change coverage. The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #164   +/-   ##
=======================================
  Coverage   83.78%   83.78%           
=======================================
  Files          25       25           
  Lines        1351     1351           
=======================================
  Hits         1132     1132           
  Misses        219      219           
PProfizi commented 2 years ago

Resolves #160.

@PProfizi, note that the extensions are different in conf.py files from dpf-post and dpf-core. I was tempted to add all extensions from dpf-core to dpf-post, but in dpf-post we have 2 that are missing in dpf-core (notfound.extension and sphinx.ext.autosummary). I do not know if this is worth an issue and try to unify the extensions criterion. Thoughts?

@GuillemBarroso IMO the plan is to homogenize these type of files as much has possible. Some extensions might be useful only in one case for a specific reason, but I do not think it hurts to add them by default in the project it is not in. It means we will be able to use the same features in both projects by default without redoing all the work. I am of the opinion of making this type of thing coherent.

PProfizi commented 2 years ago

@GuillemBarroso I do not think there is typehinting in PyDPF-Post yet, so the doc might look good now but not when actually using typehints. One thing that could be done is to choose whatever method and add typehinting to it to check how it looks in the doc?

GuillemBarroso commented 2 years ago

@PProfizi I see! Could you please point me where I can find a good typehinting example in dpf-core so I can replicate it in dpf-post?

Regarding the extensions, I was checking other repos (pymapdl and pyfluent) and they have a different extension that the one that is proposed in the documentation. The documentation proposes "sphinx.ext.autodoc.typehints", but pymapdl and pyfluent have "sphinx_autodoc_typehints", see doc on sphinx_autodoc_typehints. However, I have been trying to include "sphinx_autodoc_typehints" and the documentation file keeps being of size 7KB.

PProfizi commented 2 years ago

@PProfizi I see! Could you please point me where I can find a good typehinting example in dpf-core so I can replicate it in dpf-post?

@GuillemBarroso One of the few examples of where we actually do it in core would be here.

Regarding the extensions, I was checking other repos (pymapdl and pyfluent) and they have a different extension that the one that is proposed in the documentation. The documentation proposes "sphinx.ext.autodoc.typehints", but pymapdl and pyfluent have "sphinx_autodoc_typehints", see doc on sphinx_autodoc_typehints. However, I have been trying to include "sphinx_autodoc_typehints" and the documentation file keeps being of size 7KB.

About the documentation failing, for later reference, the documentation generation started failing on 9/24 due to a time-out during sphinx generation. trying to fix this right now. So your problem might not be due to the extension.