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

adding sphinx doc #1287

Draft
wants to merge 7 commits into
base: devel
Choose a base branch
from
Draft

adding sphinx doc #1287

wants to merge 7 commits into from

Conversation

jalalium
Copy link
Member

This commit aims to add the autogenerated sphinx documention. a build has already been performed to generate html pages. At this point, each module has its own separate page.

@jalalium jalalium added documentation needs_initial_review discussion_needed Issue cannot be tackled until we decide which way to handle it labels Dec 30, 2022
@EmilyBourne EmilyBourne marked this pull request as draft December 30, 2022 16:32
@EmilyBourne
Copy link
Member

It would be great to add a github action to build the docs.

Have you tried the numpydoc plugin yet?

@jalalium
Copy link
Member Author

jalalium commented Jan 2, 2023

It would be great to add a github action to build the docs.

Have you tried the numpydoc plugin yet?

It would be great to add a github action to build the docs.

Have you tried the numpydoc plugin yet?

Certainly, I will do so as soon as we settle on the appropriate workflow.
I tried numpydoc and it helps a lot. The docstrings that follow the standard were rendered well.
I also went ahead and added a coverage extension and got the following:

Undocumented Python objects
===========================
pyccel.ast.basic
----------------
Functions:
 * iterable

Classes:
 * Immutable
 * ScopedNode

pyccel.ast.bitwise_operators
----------------------------
Classes:
 * PyccelBitComparisonOperator
 * PyccelBitOperator

pyccel.ast.builtins
-------------------
Classes:
 * Lambda
 * PythonAbs
 * PythonComplexProperty
 * PythonSum

pyccel.ast.core
---------------
Functions:
 * apply

Classes:
 * Concatenate
 * Decorator
 * DottedFunctionCall
 * ErrorExit
 * Exit
 * FuncAddressDeclare
 * FunctionAddress
 * IfSection
 * Iterable
 * PyccelFunctionDef
 * Raise

pyccel.ast.cwrapper
-------------------
Functions:
 * C_to_Python
 * Python_to_C

Classes:
 * PyModule_AddObject

pyccel.ast.datatypes
--------------------
Classes:
 * NativeNil

pyccel.ast.headers
------------------
Classes:
 * Template

pyccel.ast.internals
--------------------
Functions:
 * symbols

pyccel.ast.literals
-------------------
Functions:
 * convert_to_literal

Classes:
 * Literal

pyccel.ast.numpy_wrapper
------------------------
Functions:
 * array_type_check
 * find_in_numpy_dtype_registry
 * scalar_type_check

pyccel.ast.numpyext
-------------------
Functions:
 * process_dtype

Classes:
 * NumpyArray
 * NumpyAutoFill
 * NumpyConjugate
 * NumpyFabs
 * NumpyHypot
 * NumpyNewArray
 * NumpyTranspose
 * NumpyUfuncBase
 * NumpyUfuncBinary
 * NumpyUfuncUnary

pyccel.ast.operators
--------------------
Functions:
 * broadcast

Classes:
 * PyccelArithmeticOperator
 * PyccelBinaryOperator
 * PyccelBooleanOperator
 * PyccelComparisonOperator
 * PyccelUnaryOperator

pyccel.ast.utilities
--------------------
Functions:
 * collect_loops
 * collect_relevant_imports
 * compatible_operation
 * expand_inhomog_tuple_assignments
 * expand_to_loops
 * get_function_from_ast
 * insert_fors
 * insert_index
 * recognised_source

Classes:
 * LoopCollection

pyccel.ast.variable
-------------------
Classes:
 * Constant
 * HomogeneousTupleVariable
 * InhomogeneousTupleVariable

pyccel.codegen.printing.luacode
-------------------------------
Functions:
 * print_lua_code

pyccel.codegen.utilities
------------------------
Functions:
 * not_a_copy

pyccel.complexity.memory
------------------------
Functions:
 * leading_term

pyccel.epyccel
--------------
Functions:
 * get_unique_name

I could use this file in the github action to ensure that all future commits will not decrease the documentation coverage.
There is also an extension for inheritence. Do you think I should add it so we get inheritence graphs as well?
On a separate note, while browsing the other doc folder, I noticed that this sphinx setup was already done a long time ago. Should we take that into account?
As for the html theme, please let me know if you prefer anotther one from these or if we should come up with our own to match Pyccel's identity.

@EmilyBourne
Copy link
Member

There is also an extension for inheritence. Do you think I should add it so we get inheritence graphs as well?

Yes, I definitely think this would be useful

@EmilyBourne
Copy link
Member

On a separate note, while browsing the other doc folder, I noticed that this sphinx setup was already done a long time ago. Should we take that into account?

I'm not sure how you want to take this into account? I suspect it was done so long ago that it needs redoing anyway. As I remember it the old docs were in the docs folder, but most of the code in there uses bad practices or doesn't work

@EmilyBourne
Copy link
Member

As for the html theme, please let me know if you prefer anotther one from these or if we should come up with our own to match Pyccel's identity.

I don't have a strong preference for any of these. They all seem fine, I don't think it's worth coming up with our own.

@EmilyBourne
Copy link
Member

I also went ahead and added a coverage extension and got the following:
I could use this file in the github action to ensure that all future commits will not decrease the documentation coverage.

I'm not sure what I am looking at here. collect_loops has a docstring. Is this all the functions/classes which do not follow the numpydoc standard?

@jalalium
Copy link
Member Author

jalalium commented Jan 2, 2023

On a separate note, while browsing the other doc folder, I noticed that this sphinx setup was already done a long time ago. Should we take that into account?

I'm not sure how you want to take this into account? I suspect it was done so long ago that it needs redoing anyway. As I remember it the old docs were in the docs folder, but most of the code in there uses bad practices or doesn't work

Okay then!

@jalalium
Copy link
Member Author

jalalium commented Jan 2, 2023

I also went ahead and added a coverage extension and got the following:
I could use this file in the github action to ensure that all future commits will not decrease the documentation coverage.

I'm not sure what I am looking at here. collect_loops has a docstring. Is this all the functions/classes which do not follow the numpydoc standard?

Exactly, althought collect_loops has a docstring, it does not follow numpydoc standard. I will fix it to showcase the standard to follow

@EmilyBourne
Copy link
Member

I also went ahead and added a coverage extension and got the following:
I could use this file in the github action to ensure that all future commits will not decrease the documentation coverage.

I'm not sure what I am looking at here. collect_loops has a docstring. Is this all the functions/classes which do not follow the numpydoc standard?

Exactly, althought collect_loops has a docstring, it does not follow numpydoc standard. I will fix it to showcase the standard to follow

Ah, that sounds very useful as an action!

@jalalium
Copy link
Member Author

jalalium commented Jan 5, 2023

The coverage actually reports the objects that were not included by the auto-doc extension. For some reason, the extention ignores certain classes and functions regardless of their doctrings. Consequently, these objects needed to be added manualy to the rst files and this is precisely what this commit serves.

pyccel.ast.utilities module
---------------------------

.. automodule:: pyccel.ast.utilities
   :members:
   :undoc-members:
   :show-inheritance:
   :private-members:
.. autoclass:: pyccel.ast.utilities.LoopCollection
   :members:
   :undoc-members:
   :show-inheritance:
   :private-members:
.. autofunction:: collect_loops
.. autofunction:: collect_relevant_imports
.. autofunction:: compatible_operation
.. autofunction:: expand_inhomog_tuple_assignments
.. autofunction:: expand_to_loops
.. autofunction:: get_function_from_ast
.. autofunction:: insert_fors
.. autofunction:: insert_index
.. autofunction:: recognised_source

In this example from docs/source/pyccel.ast.rst, automodule is not enough to detect all the functions and class that had to be added manually.

with this commit, all the modules, classes, methods, functions ... have their section in the html files but most of it still needs work. The docstring style used throughout pyccel is not consistent and most of the docstrings are not consistent with the numpydoc style nor the google style. One such inconsistency is the use of results instead of returns when describing the output of method/function. How should we go about fixing this?

In addition, some of the examples provided in the docstrings do not actually work. running make doctest will report the problems encountered. Therefore this issue should be addressed as well.

@EmilyBourne
Copy link
Member

hope should we go about fixing this

I suggest we break this down into manageable sub issues.

In theory we have been trying to use numpy doc string styles but @yguclu is the only person who actually checks for this in PRs, and we don't usually insist for small functions.

What does google doc style look like?

I suggest we proceed as follows:

  1. Open a small PR adding the action which forces the devs to use the correct doc style. This should at least prevent the situation from getting worse

  2. Open issue(s) (with tag good-first-issue) describing the problems in the current docs. To reduce the load this could be split by problem (eg results<->returns, unexecutable examples) or by file or both

  3. Go back to the doc generation

@EmilyBourne
Copy link
Member

The coverage actually reports the objects that were not included by the auto-doc extension. For some reason, the extention ignores certain classes and functions regardless of their doctrings. Consequently, these objects needed to be added manualy to the rst files and this is precisely what this commit serves.

I think it is important to understand why these classes/functions are ignored. This is a big commit though so I'm not sure where exactly you are fixing the problem. Can you link me to it so I can try and find what they have in common

@jalalium
Copy link
Member Author

jalalium commented Jan 5, 2023

Sure. To start with, running sphinx-quickstart docs will generate the necessary files. among those, two are of interest, conf.py and index.rst, The former allows to set the absolute path to the root of the project and to load extensions among other things, especially, it loads sphinx.ext.autodoc which is responsible for generating rst files for all packages in the project. To trigger its action, we use sphinx-apidoc -o source .. . this will create a template of directives for each package. For example, the following is part of pyccel.ast.rst

pyccel.ast.utilities module
---------------------------
.. automodule:: pyccel.ast.utilities
   :members:
   :undoc-members:
   :show-inheritance:
   :private-members:

now we make the docs with make html. This, however, will not include all the content of pyccel.ast.utilities module. It skips many functions such as collect_loops, collect_relevant_imports .... . And it keeps skipping them even when i changed their docstring. The only way to include them that i came up with so far is adding the following to pyccel.ast.rst

pyccel.ast.utilities module
---------------------------
.. automodule:: pyccel.ast.utilities
 :members:
 :undoc-members:
 :show-inheritance:
 :private-members:
.. autoclass:: pyccel.ast.utilities.LoopCollection
 :members:
 :undoc-members:
 :show-inheritance:
 :private-members:
.. autofunction:: collect_loops
.. autofunction:: collect_relevant_imports
.. autofunction:: compatible_operation
.. autofunction:: expand_inhomog_tuple_assignments
.. autofunction:: expand_to_loops
.. autofunction:: get_function_from_ast
.. autofunction:: insert_fors
.. autofunction:: insert_index
.. autofunction:: recognised_source

@jalalium
Copy link
Member Author

jalalium commented Jan 5, 2023

hope should we go about fixing this

I suggest we break this down into manageable sub issues.

In theory we have been trying to use numpy doc string styles but @yguclu is the only person who actually checks for this in PRs, and we don't usually insist for small functions.

What does google doc style look like?

I suggest we proceed as follows:

  1. Open a small PR adding the action which forces the devs to use the correct doc style. This should at least prevent the situation from getting worse
  2. Open issue(s) (with tag good-first-issue) describing the problems in the current docs. To reduce the load this could be split by problem (eg results<->returns, unexecutable examples) or by file or both
  3. Go back to the doc generation

hope should we go about fixing this

I suggest we break this down into manageable sub issues.

In theory we have been trying to use numpy doc string styles but @yguclu is the only person who actually checks for this in PRs, and we don't usually insist for small functions.

What does google doc style look like?

I suggest we proceed as follows:

  1. Open a small PR adding the action which forces the devs to use the correct doc style. This should at least prevent the situation from getting worse
  2. Open issue(s) (with tag good-first-issue) describing the problems in the current docs. To reduce the load this could be split by problem (eg results<->returns, unexecutable examples) or by file or both
  3. Go back to the doc generation

Here is the link to Google docstring style. This extension supports both numpy and google styles. The main difference is the naming os the sections Parameters vs Args ...
I ll create a PR for the action you mentioned.

@EmilyBourne
Copy link
Member

EmilyBourne commented Apr 14, 2023

How can I view the generated pages? Do I need to clone the repo and checkout the branch or can I see them via a link?

@jalalium
Copy link
Member Author

How can I view the generated pages? Do I need to clone the repo and checkout the branch or can I see them via a link?

At the moment, the pages are not hosted. thus you need to clone the repo and you will find them in docs/build/html.

@EmilyBourne
Copy link
Member

There is also an extension for inheritence. Do you think I should add it so we get inheritence graphs as well?

Yes, I definitely think this would be useful

@jalalium Did you ever look into this? Do you remember the name of the extension?

@jalalium
Copy link
Member Author

jalalium commented May 2, 2023

There is also an extension for inheritence. Do you think I should add it so we get inheritence graphs as well?

Yes, I definitely think this would be useful

@jalalium Did you ever look into this? Do you remember the name of the extension?

Yes, here it is. It works with the Graphviz tool. Shall I add it in this PR?

@EmilyBourne
Copy link
Member

There is also an extension for inheritence. Do you think I should add it so we get inheritence graphs as well?

Yes, I definitely think this would be useful

@jalalium Did you ever look into this? Do you remember the name of the extension?

Yes, here it is. It works with the Graphviz tool. Shall I add it in this PR?

Yes please :)

@EmilyBourne
Copy link
Member

EmilyBourne commented Aug 15, 2023

@jalalium Do you happen to still have the command that you were using to generate these docs? The doc coverage has increased quite a lot since January so hopefully the results should be improved.

It would be good to add the command to the PR somewhere too as we will need it when we are ready to do something permanent with this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion_needed Issue cannot be tackled until we decide which way to handle it documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants