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

Fix documentation #1038

Merged
merged 40 commits into from
Sep 9, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
a3b5b6f
Add flake8-rst-docstrings
Aug 30, 2021
04f2513
Add task to test the doc
Aug 28, 2021
9dc68ea
Whitelist the attribute directive
Aug 30, 2021
26b1a25
Whitelist the class role
Aug 30, 2021
f783ffd
Whitelist the meth role
Aug 30, 2021
3255f0a
Whitelist the obj role
Aug 30, 2021
f83f167
Whitelist the exc role
Aug 30, 2021
45dad35
Fix invalid markdown in rst docstring
Aug 30, 2021
cab2091
Fix invalid rst
Aug 30, 2021
178c14e
Fix malformed table
Aug 28, 2021
c2d475e
Fix reference to numpy in enums
Aug 28, 2021
fdbaf6e
Fix missing reference in simulations
Aug 28, 2021
d90b859
Fix missing reference in test runner
Aug 28, 2021
2eadc1d
Fix missing reference in tax benefit systems
Aug 28, 2021
2d71b50
Fix missing reference in parameters
Aug 28, 2021
ad9218c
Fix missing reference in variables
Aug 28, 2021
8037dca
Fix missing reference to error
Aug 28, 2021
93718b8
Fix missing reference in tax benefit systems
Aug 28, 2021
37e1980
Fix missing reference in test runner
Aug 28, 2021
2f59d9c
Fix missing reference in variables
Aug 28, 2021
de38100
Split test task in subtasks
Aug 29, 2021
a2003e0
Add option to pass the branch
Aug 29, 2021
7f671ed
Handle rebased/squashed branches
Aug 29, 2021
246bd19
Move repo to a variable
Aug 29, 2021
fb30b75
Fallback to master if no such branch
Aug 29, 2021
20d0a67
Checkout doc in circleci
Aug 29, 2021
1427760
Move test docs to its own job
Aug 29, 2021
6d19e2d
Install doc in circleci
Aug 29, 2021
23b1a81
Build doc in circleci
Aug 29, 2021
7313deb
Document how to verify that the doc build
Aug 29, 2021
5cee544
Fix wrong markup in contributing
Aug 29, 2021
3b5e95c
Add how to doc to contributing
Aug 29, 2021
7073cac
Add how to fix the doc
Aug 29, 2021
bf88c8d
Fix doc tracers
Sep 1, 2021
7b0d865
Fix doc parameters
Sep 1, 2021
bdda1a9
Fix doc variables
Sep 1, 2021
01f3193
Fix default branch to current
Sep 1, 2021
d991bdc
Change commit message in README.md
Sep 7, 2021
95e94a5
Use hyphens in Makefile
Sep 7, 2021
df51e0b
Bump minor to 35.5.0
Sep 1, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
.vscode/
build/
dist/
doc/
*.egg-info
*.mo
*.pyc
Expand Down
23 changes: 23 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
137 changes: 133 additions & 4 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

```
82 changes: 70 additions & 12 deletions Makefile
Original file line number Diff line number Diff line change
@@ -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
Loading