alan-turing-institute / PeriPy

Code base for PeriPy, a lightweight, open-source and high-performance package for peridynamic simulations written in Python - a collaboration between Exeter, Cambridge & Turing
MIT License
44 stars 15 forks source link

Upload documentation to readthedocs.org after the pull request is merged #76

Closed bb515 closed 3 years ago

bb515 commented 4 years ago

The best route for building and showing documentation looks like it is to build it with sphinx html files and upload it to readthedocs e.g. pySPH I have been following this tutorial It may turn out that admin privaleges are needed to upload the docs

bb515 commented 4 years ago

The documentation failed to build because there was no readthedocs.yaml config file, this has now been added to develop

bb515 commented 4 years ago

Problem RE meeting today

https://readthedocs.org/api/v2/build/11976984.txt

gmingas commented 4 years ago

@bb515 I had a look at readthedocs and it seems the html page is actually being built but the index.html of the page is not being built. If you type: https://peripy.readthedocs.io/en/latest/contents.html it shows a page which is the latest version (dated today after my recent push).

This is likely to be because you do not have an index.rst file within your docs/ folder. Look at this FAQ and the sphinx guide on how to build and version the .rst file here

I also noticed that there are no docstrings in the built webpage reference so might be worth inverstigating that.

gmingas commented 4 years ago

Also, not sure if the lack of dockstrings is related to the cython issue you mentioned. I could not find any error related to cython in the build log, am I missing something?

bb515 commented 3 years ago

@bb515 I had a look at readthedocs and it seems the html page is actually being built but the index.html of the page is not being built. This is likely to be because you do not have an index.rst file within your docs/ folder. Look at this [FAQ]

I had named the index rst file as 'config.rst' which, whilst it builds fine locally, sphinx searches for 'index.rst'. Thanks for spotting this, I have now ammended it. https://peripy.readthedocs.io/en/latest/

I also noticed that there are no docstrings in the built webpage reference so might be worth inverstigating that. Also, not sure if the lack of dockstrings is related to the cython issue you mentioned. I could not find any error related to cython in the build log, am I missing something?

There is a full updated issue here for @JimMadge 's reference:

The desired/expected result are full docstrings displayed in the built webpage.

The lack of docstrings is either caused by

1) the module search path to be set up properly, or 2) because the cython modules haven't been built so are not there to be found.

I think it is not caused by 1), since this answer suggests adding the line sys.path.insert(0, os.path.abspath('../')) to conf.py so that the module search path is set up properly, which results in the build: https://readthedocs.org/projects/peripy/builds/12004931/ which errors with exception

WARNING: autodoc: failed to import method 'model.this_may_take_a_while' from module 'peripy'; the following exception was raised:
No module named 'peripy.peridynamics'

So clearly the peripy. path has been correctly found, and module search path is set up properly.

This answer contrasting to the first answer I mentioned, suggests removing the line sys.path.insert(0, os.path.abspath('../')) from conf.py completely, which results in the build: https://readthedocs.org/api/v2/build/12025250.txt which errors with exception

WARNING: autodoc: failed to import module 'create_crack' from module 'peripy'; the following exception was raised:
No module named 'peripy'

So it hasn't even found the search path peripy, and module search path is not set up properly.

Therefore the lack of docstrings is caused by 2). In order to correctly build the cython modules in sphinx virtual environment, i followed this answer. In all of the recent builds, it looks like sphinx is trying to install PeriPy in the virtual environment, but the requirement is already satisfied

[rtd-command-info] start-time: 2020-10-01T15:54:24.373269Z, end-time: 2020-10-01T15:54:25.191558Z, duration: 0, exit-code: 0
/home/docs/checkouts/readthedocs.org/user_builds/peripy/envs/latest/bin/python -m pip install --upgrade --no-cache-dir pip
Requirement already up-to-date: pip in /home/docs/checkouts/readthedocs.org/user_builds/peripy/envs/latest/lib/python3.7/site-packages (20.2.3)

So the cython modules do not get built, and the peripy.peridynamics cython module is not found. I don't know how to fix this. @gmingas @JimMadge any thoughts?

Please continue use the fix/documentation_config branch for this issue, since it already has the 'contents.rst' -> 'index.rst' commit.

Edit: I had a feeling that Sphinx wasn't building the cython modules because the version number hadn't changed, as it thought it had already installed the correct version of the package. I tried changing the version number on abed3f8dffec576e5ca1e0e475510b9fe9286965 but when building on sphinx, no sign of the package building properly on the raw build log and it errors with the same 'No module named 'peripy.peridynamics'. So this didn't fix the issue.

JimMadge commented 3 years ago

From the error message, I think it's most likely that sphinx (as run by readthedocs) is not able to import peripy.

Does sphinx run correctly on a local clone of the repository? If so, the challenge is probably to find out exactly what sphinx command readthedocs is running (and in what directory) and how that is different.

bb515 commented 3 years ago

I had fixed this in #96. The problems were: Need to include Cython as a requirement in requirements.txt Need to include the lines

  install:
    - requirements: docs/requirements.txt
    - method: setuptools
      path: .

in .readthedocs.yaml So that sphinx does this all in the correct order. In general there seems to be some unclear conflicting information on how is best to achieve this but the solution turned out to be simple.