Fix documentation

Merge pull request #1038 from openfisca/fix-doc

.circleci/config.yml

@@ -48,6 +48,44 @@ jobs:
             COUNTRY_TEMPLATE_PATH=`python -c "import openfisca_country_template; print(openfisca_country_template.CountryTaxBenefitSystem().get_package_metadata()['location'])"`
             openfisca test $COUNTRY_TEMPLATE_PATH/openfisca_country_template/tests/
 
+  test_docs:
+    docker:
+      - image: python:3.7
+
+    steps:
+      - checkout
+
+      - run:
+          name: Checkout docs
+          command: make test-doc-checkout branch=$CIRCLE_BRANCH
+
+      - restore_cache:
+          key: v1-py3-{{ .Branch }}-{{ checksum "setup.py" }}
+
+      - restore_cache:
+          key: v1-py3-docs-{{ .Branch }}-{{ checksum "doc/requirements.txt" }}
+
+      - run:
+          name: Create a virtualenv
+          command: |
+            mkdir -p /tmp/venv/openfisca_doc
+            python -m venv /tmp/venv/openfisca_doc
+            echo "source /tmp/venv/openfisca_doc/bin/activate" >> $BASH_ENV
+
+      - run:
+          name: Install dependencies
+          command: make test-doc-install
+
+      - save_cache:
+          key: v1-py3-docs-{{ .Branch }}-{{ checksum "doc/requirements.txt" }}
+          paths:
+            - /tmp/venv/openfisca_doc
+
+      - run:
+          name: Run doc tests
+          command: make test-doc-build
+
+
   check_version:
     docker:
       - image: python:3.7
@@ -123,13 +161,15 @@ workflows:
   build_and_deploy:
     jobs:
       - run_tests
+      - test_docs
       - check_version
       - submit_coverage:
           requires:
             - run_tests
       - deploy:
           requires:
             - run_tests
+            - test_docs
             - check_version
           filters:
             branches:

.gitignore

@@ -7,6 +7,7 @@
 .vscode/
 build/
 dist/
+doc/
 *.egg-info
 *.mo
 *.pyc

CHANGELOG.md

@@ -1,5 +1,28 @@
 # Changelog
 
+## 35.5.0 [#1038](https://github.com/openfisca/openfisca-core/pull/1038)
+
+#### New Features
+
+- Introduce `openfisca_core.variables.typing`
+  - Documents the signature of formulas
+  - Note: as formulas are generated dynamically, documenting them is tricky
+
+#### Bug Fixes
+
+- Fix the official doc
+  - Corrects malformed docstrings
+  - Fix missing and/ou outdated references
+
+#### Technical Changes
+
+- Add tasks to automatically validate that changes do not break the documentation
+
+#### Documentation
+
+- Add steps to follow in case the documentation is broken
+- Add general documenting guidelines in CONTRIBUTING.md
+
 ### 35.4.2 [#1026](https://github.com/openfisca/openfisca-core/pull/1026)
 
 #### Bug fix

CONTRIBUTING.md

@@ -65,9 +65,138 @@ We strive to deliver great error messages, which means they are:
 
 ### Example
 
-> **Terrible**: `unexpected value`.
-> **Bad**: `argument must be a string, an int, or a period`.
-> **Good**: `Variable {0} has already been set for all months contained in period {1}, and value {2} provided for {1} doesn't match the total ({3}).`
-> **Great**: `Inconsistent input: variable {0} has already been set for all months contained in period {1}, and value {2} provided for {1} doesn't match the total ({3}). This error may also be thrown if you try to call set_input twice for the same variable and period. See more at <https://openfisca.org/doc/key-concepts/periodsinstants.html>.`
+- **Terrible**: `unexpected value`.
+- **Bad**: `argument must be a string, an int, or a period`.
+- **Good**: `Variable {0} has already been set for all months contained in period {1}, and value {2} provided for {1} doesn't match the total ({3}).`
+- **Great**: `Inconsistent input: variable {0} has already been set for all months contained in period {1}, and value {2} provided for {1} doesn't match the total ({3}). This error may also be thrown if you try to call set_input twice for the same variable and period. See more at <https://openfisca.org/doc/key-concepts/periodsinstants.html>.`
 
 [More information](https://blogs.mulesoft.com/dev/api-dev/api-best-practices-response-handling/).
+
+## Documentation
+
+OpenFisca does not yet follow a common convention for docstrings, so you'll find [ReStructuredText](https://www.sphinx-doc.org/en/master/usage/restructuredtext/domains.html), [Google](http://google.github.io/styleguide/pyguide.html#Comments), and [NumPy](https://github.com/numpy/numpy/blob/master/doc/HOWTO_DOCUMENT.rst.txt) style docstrings.
+
+Whatever the style you choose, contributors and reusers alike will be more than thankful for the effort you put in documenting your contributions. Here are some general good practices you can follow:
+
+1. TL;DR: Document your intent :smiley:
+
+2. When adding a new module with several classes and functions, please structure it in different files, create an `__init__.py` file and document it as follows:
+
+```py
+"""Short summary of your module.
+
+A longer description of what are your module's motivation, domain, and use 
+cases. For example, if you decide to create a caching system for OpenFisca, 
+consisting on different caching mechanisms, you could say that large operations 
+are expensive for some users, that different caching mechanisms exist, and that 
+this module implements some of them.
+
+You can then give examples on how to use your module:
+
+    .. code-block:: python
+
+        this = cache(result)
+        that = this.flush()
+
+Better you can write `doctests` that will be run with the test suite:
+
+    >>> from . import Cache
+    >>> cache = Cache("VFS")
+    >>> cache.add("ratio", .45)
+    >>> cache.get("ratio")
+    .45
+
+"""
+
+from .cache import Cache
+from .strategies import Memory, Disk
+
+__all__ = ["Cache", "Memory", "Disk"]
+```
+
+3. When adding a new class, you can either document the class itself, the `__init__` method, or both:
+
+```py
+class Cache:
+    """Implements a new caching system.
+
+    Same as before, you could say this is good because virtuals systems are 
+    great but the need a wrapper to make them work with OpenFisca.
+
+    Document the class attributes —different from the initialisation arguments:
+        type (str): Type of cache.
+        path (str): An so on…
+
+    Document the class arguments, here or in the `__init__` method:
+        type: For example if you need a ``type`` to create the :class:`.Cache`.
+
+    Please note that if you're using Python type annotations, you don't need to
+    specify types in the documentation, they will be parsed and verified
+    automatically.
+
+    """
+
+    def __init__(self, type: str) -> None:
+        pass
+
+```
+
+4. Finally, when adding methods to your class, or helper functions to your module, it is very important to document their contracts:
+
+```py
+def get(self, key: str) -> Any:
+    """Again, summary description.
+
+    The long description is optional, as long as the code is easy to 
+    understand. However, there are four key elements to help others understand
+    what the code does:
+
+        * What it takes as arguments
+        * What it returns
+        * What happens if things fail
+        * Valid examples that can be run!
+
+    For example if we were following the Google style, it would look like this:
+
+    Args:
+        key: The ``key`` to retrieve from the :obj:`.Cache`.
+
+    Returns:
+        Whatever we stored, if we stored it (see, no need to specify the type)
+
+    Raises:
+        :exc:`KeyNotFoundError`: When the ``key`` wasn't found.
+
+    Examples:
+        >>> cache = Cache()
+        >>> cache.set("key", "value")
+        >>> cache.get("key")
+        "value"
+
+    Note:
+        Your examples should be simple and illustrative. For more complex
+        scenarios write a regular test.
+
+    Todo:
+        * Accept :obj:`int` as ``key``.
+        * Return None when key is not found.
+
+    .. versionadded:: 1.2.3
+        This will help people to undestand the code evolution.
+
+    .. deprecated:: 2.3.4
+        This, to have time to adapt their own codebases before the code is
+        removed forever.
+
+    .. seealso::
+        Finally, you can help users by referencing useful documentation of
+        other code, like :mod:`numpy.linalg`, or even links, like the official
+        OpenFisca `documentation`_.
+
+    .. _documentation: https://openfisca.org/doc/
+
+    """
+
+    pass
+
+```

Makefile

@@ -1,62 +1,120 @@
-doc = sed -n "/^$1/ { x ; p ; } ; s/\#\#/[⚙]/ ; s/\./.../ ; x" ${MAKEFILE_LIST}
+help = sed -n "/^$1/ { x ; p ; } ; s/\#\#/[⚙]/ ; s/\./.../ ; x" ${MAKEFILE_LIST}
+repo = https://github.com/openfisca/openfisca-doc
+branch = $(shell git branch --show-current)
 
-## Same as `make test.
+## Same as `make test`.
 all: test
 
 ## Install project dependencies.
 install:
-	@$(call doc,$@:)
+	@$(call help,$@:)
 	@pip install --upgrade pip twine wheel
 	@pip install --editable .[dev] --upgrade --use-deprecated=legacy-resolver
 
 ## Install openfisca-core for deployment and publishing.
 build: setup.py
 	@## This allows us to be sure tests are run against the packaged version
 	@## of openfisca-core, the same we put in the hands of users and reusers.
-	@$(call doc,$@:)
+	@$(call help,$@:)
 	@python $? bdist_wheel
 	@find dist -name "*.whl" -exec pip install --force-reinstall {}[dev] \;
 
 ## Uninstall project dependencies.
 uninstall:
-	@$(call doc,$@:)
+	@$(call help,$@:)
 	@pip freeze | grep -v "^-e" | sed "s/@.*//" | xargs pip uninstall -y
 
 ## Delete builds and compiled python files.
 clean: \
 	$(shell ls -d * | grep "build\|dist") \
 	$(shell find . -name "*.pyc")
-	@$(call doc,$@:)
+	@$(call help,$@:)
 	@rm -rf $?
 
 ## Compile python files to check for syntax errors.
 check-syntax-errors: .
-	@$(call doc,$@:)
+	@$(call help,$@:)
 	@python -m compileall -q $?
 
 ## Run linters to check for syntax and style errors.
 check-style: $(shell git ls-files "*.py")
-	@$(call doc,$@:)
+	@$(call help,$@:)
 	@flake8 $?
 
 ## Run code formatters to correct style errors.
 format-style: $(shell git ls-files "*.py")
-	@$(call doc,$@:)
+	@$(call help,$@:)
 	@autopep8 $?
 
 ## Run static type checkers for type errors.
 check-types: openfisca_core openfisca_web_api
-	@$(call doc,$@:)
+	@$(call help,$@:)
 	@mypy $?
 
 ## Run openfisca-core tests.
 test: clean check-syntax-errors check-style check-types
-	@$(call doc,$@:)
+	@$(call help,$@:)
 	@env PYTEST_ADDOPTS="${PYTEST_ADDOPTS} --cov=openfisca_core" pytest
 
+## Check that the current changes do not break the doc.
+test-doc:
+	@##	Usage:
+	@##
+	@##		make test-doc [branch=BRANCH]
+	@##
+	@##	Examples:
+	@##
+	@##		# Will check the current branch in openfisca-doc.
+	@##		make test-doc
+	@##
+	@##		# Will check "test-doc" in openfisca-doc.
+	@##		make test-doc branch=test-doc
+	@##
+	@##		# Will check "master" if "asdf1234" does not exist.
+	@##		make test-doc branch=asdf1234
+	@##
+	@$(call help,$@:)
+	@${MAKE} test-doc-checkout
+	@${MAKE} test-doc-install
+	@${MAKE} test-doc-build
+
+## Update the local copy of the doc.
+test-doc-checkout:
+	@$(call help,$@:)
+	@[ ! -d doc ] && git clone ${repo} doc || :
+	@cd doc && { \
+		git reset --hard ; \
+		git fetch --all ; \
+		[ $$(git branch --show-current) != master ] && git checkout master || : ; \
+		[ ${branch} != "master" ] \
+			&& { \
+				{ \
+					git branch -D ${branch} 2> /dev/null ; \
+					git checkout ${branch} ; \
+				} \
+					&& git pull --ff-only origin ${branch} \
+					|| { \
+						>&2 echo "[!] The branch '${branch}' doesn't exist, checking out 'master' instead..." ; \
+						git pull --ff-only origin master ; \
+					} \
+			} \
+			|| git pull --ff-only origin master ; \
+	} 1> /dev/null
+
+## Install doc dependencies.
+test-doc-install:
+	@$(call help,$@:)
+	@pip install --requirement doc/requirements.txt 1> /dev/null
+	@pip install --editable .[dev] --upgrade 1> /dev/null
+
+## Dry-build the doc.
+test-doc-build:
+	@$(call help,$@:)
+	@sphinx-build -M dummy doc/source doc/build -n -q -W
+
 ## Serve the openfisca Web API.
 api:
-	@$(call doc,$@:)
+	@$(call help,$@:)
 	@openfisca serve \
 		--country-package openfisca_country_template \
 		--extensions openfisca_extension_template