From a39721f611d58c3cf2d80c8135e27d7efe238b27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Robert?= Date: Thu, 16 Jul 2020 23:19:37 +0200 Subject: [PATCH 1/3] add black and pre-commit hooks sections to CONTRIBUTING.rst --- CONTRIBUTING.rst | 59 +++++++++++++++++++++++++++++++----------------- 1 file changed, 38 insertions(+), 21 deletions(-) diff --git a/CONTRIBUTING.rst b/CONTRIBUTING.rst index 1b148aea4f1..9070877b677 100644 --- a/CONTRIBUTING.rst +++ b/CONTRIBUTING.rst @@ -716,39 +716,42 @@ active PR for which ``feature_1`` is a pointer to the ``HEAD`` commit. A push to Coding Style Guide ================== -Automatically checking code style ---------------------------------- +Automatically checking and fixing code style +-------------------------------------------- Below are a list of rules for coding style in yt. Some of these rules are suggestions are not explicitly enforced, while some are enforced via automated -testing. The yt project uses a subset of the rules checked by ``flake8`` to -verify our code. The ``flake8`` tool is a combination of the ``pyflakes`` and -``pep8`` tools. To check the coding style of your contributions locally you will -need to install the ``flake8`` tool from ``pip``: +testing. + +The yt project uses ``flake8`` to report on code correctness (syntax + anti-pattern +detection), and ``black`` for automated formatting. + +To check the coding style of your contributions locally you will need to install those +tools, which can be done for instance with ``pip``: .. code-block:: bash - $ pip install flake8 + $ pip install tests/lint_requirements.txt -And then navigate to the root of the yt repository and run ``flake8`` on the -``yt`` subdirectory: +Then run the checks from the top level of the repository with .. code-block:: bash - $ cd yt-git - $ flake8 ./yt + $ flake8 yt/ + $ black --check + +These will respectively print out any ``flake8`` errors or warnings that your newly added +code triggers, and a list of files that are currenlty not compliant with ``black``. Note +that only a subset of the `full flake8 error and warning list +`_ is run, since we +explicitly blacklist some of rules that are checked by ``flake8`` by default. -This will print out any ``flake8`` errors or warnings that your newly added code -triggers. The errors will be in your newly added code because we have already -cleaned up the rest of the yt codebase of the errors and warnings detected by -the `flake8` tool. Note that this will only trigger a subset of the `full flake8 -error and warning list -`_, since we explicitly -blacklist a large number of the full list of rules that are checked by -``flake8`` by default. +Run black without the ``--check`` flag to automatically update the code to a +black-compliant form. -Import Formatting ------------------ + +Import ordering +--------------- We use ``isort`` to enforce PEP-8 guidelines for import ordering. By decreasing priority order: @@ -768,6 +771,20 @@ To validate import order, run ``isort`` recursively at the top level If any error is detected, rerun this without the ``--check-only`` flag to fix them. +Pre-commit hooks +---------------- + +If you wish to automate this process you may be interested in using `pre-commit +`_ hooks. They can be installed from the repo's top level with + +.. code-block:: bash + + $ pre-commit install + +So that ``black``, ``flake8`` and ``isort`` will run and update your changes every time +you commit new code. This setup is not required so you have the option of checking for +code style only in the late stage of a branch when we need to validate it for merging. + Source code style guide ----------------------- From cf5427c1c362b3ed7183d4671b445959e7d1cbb9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Robert?= Date: Thu, 16 Jul 2020 23:24:02 +0200 Subject: [PATCH 2/3] fix typos --- CONTRIBUTING.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CONTRIBUTING.rst b/CONTRIBUTING.rst index 9070877b677..3833d9d988e 100644 --- a/CONTRIBUTING.rst +++ b/CONTRIBUTING.rst @@ -744,10 +744,10 @@ These will respectively print out any ``flake8`` errors or warnings that your ne code triggers, and a list of files that are currenlty not compliant with ``black``. Note that only a subset of the `full flake8 error and warning list `_ is run, since we -explicitly blacklist some of rules that are checked by ``flake8`` by default. +explicitly blacklist some of the rules that are checked by ``flake8`` by default. Run black without the ``--check`` flag to automatically update the code to a -black-compliant form. +``black``-compliant form. Import ordering From 22e5184dc372a065677bf5334d6d05a860f108a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Robert?= Date: Fri, 17 Jul 2020 10:28:06 +0200 Subject: [PATCH 3/3] doc: some simplifications in existing guidelines --- CONTRIBUTING.rst | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/CONTRIBUTING.rst b/CONTRIBUTING.rst index 3833d9d988e..68fbf28a220 100644 --- a/CONTRIBUTING.rst +++ b/CONTRIBUTING.rst @@ -814,15 +814,11 @@ Source code style guide of a docstring. * Use only one top-level import per line. Unless there is a good reason not to, imports should happen at the top of the file. - * Never compare with ``True`` or ``False`` using ``==`` or ``!=``, always use - ``is`` or ``is not``. + * Never compare with singleton ``True``, ``False``, ``None`` ... using ``==`` or ``!=``, + always use ``is`` or ``is not``. * If you are comparing with a numpy boolean array, just refer to the array. Ex: do ``np.all(array)`` instead of ``np.all(array == True)``. - * Never compare with None using ``==`` or ``!=``, use ``is None`` or - ``is not None``. * Use ``statement is not True`` instead of ``not statement is True`` - * Only one statement per line, do not use semicolons to put two or more - statements on a single line. * Only declare local variables if they will be used later. If you do not use the return value of a function, do not store it in a variable. * Add tests for new functionality. When fixing a bug, consider adding a test to @@ -848,14 +844,12 @@ API Style Guide * Do not use too many keyword arguments. If you have a lot of keyword arguments, then you are doing too much in ``__init__`` and not enough via parameter setting. - * In function arguments, place spaces before commas. ``def something(a,b,c)`` - should be ``def something(a, b, c)``. * Don't create a new class to replicate the functionality of an old class -- replace the old class. Too many options makes for a confusing user experience. * Parameter files external to yt are a last resort. * The usage of the ``**kwargs`` construction should be avoided. If they cannot - be avoided, they must be explained, even if they are only to be passed on to + be avoided, they must be documented, even if they are only to be passed on to a nested function. .. _docstrings: