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 fixes #463

Merged
merged 7 commits into from
Jun 7, 2021
Merged

Doc fixes #463

merged 7 commits into from
Jun 7, 2021

Conversation

brandonwillard
Copy link
Member

This PR contains numerous Sphinx-related documentation fixes. Only one set of warnings remain after these changes, and fixing those involves a refactor of Type.[Variable|Constant].

@brandonwillard brandonwillard self-assigned this Jun 6, 2021
@brandonwillard brandonwillard added documentation Improvements or additions to documentation enhancement New feature or request labels Jun 6, 2021
@codecov
Copy link

codecov bot commented Jun 6, 2021

Codecov Report

Merging #463 (eeb84b0) into main (eb4f92f) will decrease coverage by 0.00%.
The diff coverage is 88.23%.

❗ Current head eeb84b0 differs from pull request most recent head 0bb28cb. Consider uploading reports for the commit 0bb28cb to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             main     #463      +/-   ##
==========================================
- Coverage   72.52%   72.52%   -0.01%     
==========================================
  Files         174      174              
  Lines       55689    55680       -9     
==========================================
- Hits        40390    40381       -9     
  Misses      15299    15299              
Impacted Files Coverage Δ
aesara/gpuarray/dnn.py 23.78% <ø> (ø)
aesara/graph/basic.py 91.79% <ø> (ø)
aesara/graph/fg.py 93.22% <ø> (ø)
aesara/graph/op.py 76.90% <ø> (-0.06%) ⬇️
aesara/graph/type.py 84.07% <ø> (ø)
aesara/link/basic.py 89.23% <ø> (ø)
aesara/link/c/interface.py 94.04% <ø> (ø)
aesara/link/jax/dispatch.py 82.05% <ø> (ø)
aesara/scalar/math.py 77.18% <ø> (ø)
aesara/sparse/basic.py 88.96% <ø> (ø)
... and 7 more

Copy link
Contributor

@michaelosthege michaelosthege left a comment

Choose a reason for hiding this comment

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

Good stuff - looks like a lot of work!

I noticed some inconsistency in the spaces and colons on the type hints in docstrings. The linked resources could help to explain & resolve them.

aesara/graph/basic.py Outdated Show resolved Hide resolved
aesara/graph/basic.py Outdated Show resolved Hide resolved
aesara/graph/fg.py Outdated Show resolved Hide resolved
aesara/graph/fg.py Outdated Show resolved Hide resolved
aesara/graph/op.py Outdated Show resolved Hide resolved
@brandonwillard
Copy link
Member Author

brandonwillard commented Jun 6, 2021

Good stuff - looks like a lot of work!

I noticed some inconsistency in the spaces and colons on the type hints in docstrings. The linked resources could help to explain & resolve them.

These changes come from an accumulation of doc-related stashes over the past few months. Those empty :s were there to indicate portions of the docstrings that still needed to be filled out; however, now that we've been adding type annotations, I would rather not maintain two copies of the same information, and simply automate the docstring parameters production via the type annotations (e.g. sphinx-autodoc-typehints).

With that in mind, we should focus less on manually meeting the subtleties of NumPy's parameter formatting, since it will ultimately be a wasted effort. Likewise, the entire project doesn't adhere to a single, consistent docstring format, but, once one is formally established, that would entail a pre-commit linter and updates to all the existing docstrings. This can—and should—be done independently.

In the meantime, we need to fix the obvious formatting issues in the rendered documentation, and that's what this PR primarily does.

michaelosthege
michaelosthege previously approved these changes Jun 7, 2021
Copy link
Contributor

@michaelosthege michaelosthege left a comment

Choose a reason for hiding this comment

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

[...] I would rather not maintain two copies of the same information, and simply automate the docstring parameters production via the type annotations [...]
In the meantime, we need to fix the obvious formatting issues in the rendered documentation, and that's what this PR primarily does.

I agree! Just wanted to make sure that you're aware of the potentially related issue.

doc/extending/extending_aesara.rst Outdated Show resolved Hide resolved
@brandonwillard brandonwillard merged commit 3a3adae into aesara-devs:main Jun 7, 2021
@brandonwillard brandonwillard deleted the doc-fixes branch June 7, 2021 21:55
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 enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants