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

Issue277 auto generate api documentation #280

Merged
merged 18 commits into from
Jun 24, 2019

Conversation

juhoinkinen
Copy link
Member

@juhoinkinen juhoinkinen commented May 31, 2019

Initial auto-generated API documentation with Sphinx using readthedocs theme.

Running make html in docs/ creates docs/_build/html directory based on the .rst files in docs/source. The index.html is the main page to start navigation.

Currently there is essentially only autogenerated API information, just version number and few links in the index page are manually added. Should there be anything else at this point?

The .rts files are originally generated with sphinx-apidoc -o source/ ../annif. Now make html is enough to get additions and changes from source code docstrings to documentation.

Edit: If a package/module is added, it is necessary to overwrite the existing .rst files by running sphinx-apidoc -f -o source/ ../annif in docs/ before make html to get the changes.

Uploading docs to readthedocs.io should be easy, they too are essentially running make html (the .rst files need to be in the repository):
https://docs.readthedocs.io/en/stable/builds.html#how-we-build-documentation

(closes #277 )

@codecov
Copy link

codecov bot commented May 31, 2019

Codecov Report

Merging #280 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #280   +/-   ##
=======================================
  Coverage   99.31%   99.31%           
=======================================
  Files          52       52           
  Lines        2624     2624           
=======================================
  Hits         2606     2606           
  Misses         18       18

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ffc947f...21a2fb9. Read the comment docs.

@osma
Copy link
Member

osma commented Jun 4, 2019

I tried running make html but got this error:

(Annif-OYFUWV2R) oisuomin@dx2-lib-2:~/git/Annif/docs$ make html
Running Sphinx v1.3.6
loading pickled environment... not yet created
building [mo]: targets for 0 po files that are out of date
building [html]: targets for 6 source files that are out of date
updating environment: 6 added, 0 changed, 0 removed
reading sources... [100%] source/modules                                                                                                                                                                                                     

Sphinx error:
master file /home/oisuomin/git/Annif/docs/contents.rst not found
Makefile:19: recipe for target 'html' failed
make: *** [html] Error 1

This was after installing the new dependencies with pipenv install and entering pipenv shell.
Am I doing something wrong? Should the file docs/contents.rst be in the repo too?

@juhoinkinen
Copy link
Member Author

juhoinkinen commented Jun 5, 2019

The above error could be related to some version issues, see readthedocs/readthedocs.org#2569

Tried to fix by setting master_doc = 'index' in conf.py

@osma
Copy link
Member

osma commented Jun 5, 2019

Tested this again. The contents.rst error is now gone. Instead getting import errors like this:

Running Sphinx v1.3.6
loading pickled environment... done
building [mo]: targets for 0 po files that are out of date
building [html]: targets for 0 source files that are out of date
updating environment: 0 added, 4 changed, 0 removed
reading sources... [ 25%] source/annif
reading sources... [ 50%] source/annif.analyzer
reading sources... [ 75%] source/annif.backend
reading sources... [100%] source/annif.corpus

/home/local/oisuomin/git/Annif/docs/source/annif.rst:13: WARNING: autodoc: failed to import module u'annif.cli'; the following exception was raised:
Traceback (most recent call last):
  File "/usr/lib/python2.7/dist-packages/sphinx/ext/autodoc.py", line 386, in import_object
    __import__(self.modname)
  File "/home/local/oisuomin/git/Annif/annif/__init__.py", line 6, in <module>
    import connexion
ImportError: No module named connexion
/home/local/oisuomin/git/Annif/docs/source/annif.rst:21: WARNING: autodoc: failed to import module u'annif.datadir'; the following exception was raised:
Traceback (most recent call last):
  File "/usr/lib/python2.7/dist-packages/sphinx/ext/autodoc.py", line 386, in import_object
    __import__(self.modname)
  File "/home/local/oisuomin/git/Annif/annif/__init__.py", line 6, in <module>
    import connexion
ImportError: No module named connexion

This was after running pipenv install and entering pipenv shell, where annif itself seems to be working.

The HTML docs were generated but in practice they were pretty much empty because the docstrings couldn't be loaded due to the import errors.

Anyway the next step would probably be to set up docs generation under ReadTheDocs (and possibly also Travis CI, by adding the necessary commands to .travis.yml)

@juhoinkinen
Copy link
Member Author

Anyway the next step would probably be to set up docs generation under ReadTheDocs (and possibly also Travis CI, by adding the necessary commands to .travis.yml)

The docs are now publicly available in in https://annif.readthedocs.io/en/latest/index.html. Currently all pushes to this branch (issue277-...) trigger building and publishing the docs. Travis is not involved in this, but the webhook system of readthedocs takes care of everything.

TODO: Switch the trigger to listen to pushes to master (when this branch is ready to be merged, https://readthedocs.org/dashboard/annif/advanced/). Or, maybe the docs should be updated only on new release?

Unfortunately, at the moment for building docs in readthedocs requirements.txt or requirements-dev.txt is needed (hence those files and pipenv-to-requirements package). However, Pipfiles are going to be supported, and the requirements files can be dropped.

@juhoinkinen
Copy link
Member Author

/home/local/oisuomin/git/Annif/docs/source/annif.rst:13: WARNING: autodoc: failed to import module u'annif.cli'; the following exception was raised:
Traceback (most recent call last):
File "/usr/lib/python2.7/dist-packages/sphinx/ext/autodoc.py", line 386, in import_object
import(self.modname)
File "/home/local/oisuomin/git/Annif/annif/init.py", line 6, in
import connexion
ImportError: No module named connexion

Maybe the import errors were due to Sphinx wrongly using Python2 for some reason, as this looks like.

@osma
Copy link
Member

osma commented Jun 19, 2019

The docs are now publicly available in in https://annif.readthedocs.io/en/latest/index.html. Currently all pushes to this branch (issue277-...) trigger building and publishing the docs

Excellent!

Travis is not involved in this, but the webhook system of readthedocs takes care of everything.

This is OK. We don't need to involve Travis here.

Or, maybe the docs should be updated only on new release?

RTD docs are versioned, right? So it should be possible to have docs built both for the current master branch as well as any tagged releases. I think that's what we want.

Edit: If a package/module is added, it is necessary to overwrite the existing .rst files by running sphinx-apidoc -f -o source/ ../annif in docs/ before make html to get the changes.

Would it be easy to add a rule to do this in the Makefile? The documentation should track changes in package/module structure as automatically as possible.

I don't see a RTD config file included in the PR. Should we have one, even a minimal one? According to the link, "Using a configuration file is the recommended way of using Read the Docs."

Unfortunately, at the moment for building docs in readthedocs requirements.txt or requirements-dev.txt is needed

Are you sure that we really need a requirements.txt file? The dependencies for Annif are already specified in setup.py (install_requires) which makes it possible to install Annif e.g. from PyPI using pip. In my understanding RTD uses pip as well, so it should find and install the dependencies in setup.py as well?

@osma osma mentioned this pull request Jun 24, 2019
@osma
Copy link
Member

osma commented Jun 24, 2019

I'll merge this now. I'm sure we can work out any outstanding issues in follow-up PRs.

@osma osma merged commit 5e2331f into master Jun 24, 2019
@juhoinkinen
Copy link
Member Author

juhoinkinen commented Jun 25, 2019

Thanks, good have this finished!

Or, maybe the docs should be updated only on new release?

RTD docs are versioned, right? So it should be possible to have docs built both for the current master branch as well as any tagged releases. I think that's what we want.

Yes, there's latest and stable versions following master branch and tagged releases, respectively, which come by default: https://docs.readthedocs.io/en/stable/versions.html#versions

TODO: Switch the trigger to listen to pushes to master (when this branch is ready to be merged, https://readthedocs.org/dashboard/annif/advanced/).

NB: Done

Unfortunately, at the moment for building docs in readthedocs requirements.txt or requirements-dev.txt is needed

Are you sure that we really need a requirements.txt file? The dependencies for Annif are already specified in setup.py (install_requires) which makes it possible to install Annif e.g. from PyPI using pip. In my understanding RTD uses pip as well, so it should find and install the dependencies in setup.py as well?

You are right, there is an option for RTD builds which makes requirements.txt unnecessary (in Avanced Settings):

Install your project inside a virtualenv using setup.py install

I created a separate PR #285 for removing the unnecessary requirements files.

One final point: @osma I think you too should have admin rights to the annif project in readthedocs. There is a user osma in readthedocs, is that you? If yes I'll add that user (or maybe the admin rights come automatically via github when logging in RTD, but at least now there's only me in the admin list).

@juhoinkinen juhoinkinen deleted the issue277-auto-generate-api-documentation branch August 15, 2019 13:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Auto-generate API documentation
2 participants