Merge pull request #2702 from pybamm-team/pre-commit

Author: valentinsulzer

.github/PULL_REQUEST_TEMPLATE.md

@@ -14,11 +14,11 @@ Please add a line in the relevant section of [CHANGELOG.md](https://github.com/p
 
 # Key checklist:
 
-- [ ] No style issues: `$ pre-commit run`
-- [ ] All tests pass: `$ python run-tests.py --unit`
-- [ ] The documentation builds: `$ cd docs` and then `$ make clean; make html`
+- [ ] No style issues: `$ pre-commit run` (see [CONTRIBUTING.md](https://github.com/pybamm-team/PyBaMM/blob/develop/CONTRIBUTING.md#installing-and-using-pre-commit) for how to set this up to run automatically when committing locally, in just two lines of code)
+- [ ] All tests pass: `$ python run-tests.py --all`
+- [ ] The documentation builds: `$ python run-tests.py --doctest`
 
-You can run all three at once, using `$ python run-tests.py --quick`.
+You can run unit and doctests together at once, using `$ python run-tests.py --quick`.
 
 ## Further checks:
 

.pre-commit-config.yaml

@@ -3,12 +3,6 @@ ci:
   autofix_commit_msg: "style: pre-commit fixes"
 
 repos:
-  - repo: https://github.com/pre-commit/mirrors-prettier
-    rev: v3.0.0-alpha.4
-    hooks:
-      - id: prettier
-        exclude: assets/js/webapp\.js
-
   - repo: https://github.com/psf/black
     rev: 23.1.0
     hooks:

CONTRIBUTING.md

@@ -61,7 +61,7 @@ Finally, if you really, really, _really_ love developing PyBaMM, have a look at
 
 ## Coding style guidelines
 
-PyBaMM follows the [PEP8 recommendations](https://www.python.org/dev/peps/pep-0008/) for coding style. These are very common guidelines, and community tools have been developed to check how well projects implement them. We recommend using pre-commit hooks to check your code before committing it. See [installing and using pre-commit](https://github.com/pybamm-team/PyBaMM/blob/develop/CONTRIBUTING.md#installing-and-using-pre-commit) section for more details.
+PyBaMM follows the [PEP8 recommendations](https://www.python.org/dev/peps/pep-0008/) for coding style. These are very common guidelines, and community tools have been developed to check how well projects implement them. We recommend using pre-commit hooks to check your code before committing it. See [installing and using pre-commit](#installing-and-using-pre-commit) section for more details.
 
 ### Ruff
 
@@ -307,7 +307,7 @@ PyBaMM is documented in several ways.
 
 First and foremost, every method and every class should have a [docstring](https://www.python.org/dev/peps/pep-0257/) that describes in plain terms what it does, and what the expected input and output is.
 
-These docstrings can be fairly simple, but can also make use of [reStructuredText](http://docutils.sourceforge.net/docs/user/rst/quickref.html), a markup language designed specifically for writing [technical documentation](https://en.wikipedia.org/wiki/ReStructuredText). For example, you can link to other classes and methods by writing ``:class:`pybamm.Model` `` and ``:meth:`run()` ``.
+These docstrings can be fairly simple, but can also make use of [reStructuredText](http://docutils.sourceforge.net/docs/user/rst/quickref.html), a markup language designed specifically for writing [technical documentation](https://en.wikipedia.org/wiki/ReStructuredText). For example, you can link to other classes and methods by writing `` :class:`pybamm.Model`  `` and `` :meth:`run()`  ``.
 
 In addition, we write a (very) small bit of documentation in separate reStructuredText files in the `docs` directory. Most of what these files do is simply import docstrings from the source code. But they also do things like add tables and indexes. If you've added a new class to a module, search the `docs` directory for that module's `.rst` file and add your class (in alphabetical order) to its index. If you've added a whole new module, copy-paste another module's file and add a link to your new file in the appropriate `index.rst` file.
 

docs/source/user_guide/installation/GNU-linux.rst

@@ -197,12 +197,6 @@ not being used when I run my Python script.
 i.e. ``pip install -e .``. This sets the installed location of the
 source files to your current directory.
 
-**Problem:** When running ``python run-tests.py --quick``, gives error
-``FileNotFoundError: [Errno 2] No such file or directory: 'flake8': 'flake8``.
-
-**Solution:** make sure you have included the ``[dev,docs]`` flags when
-you pip installed PyBaMM, i.e. ``pip install -e .[dev,docs]``
-
 **Problem:** Errors when solving model
 ``ValueError: Integrator name ida does not exsist``, or
 ``ValueError: Integrator name cvode does not exsist``.

docs/source/user_guide/installation/install-from-source.rst

@@ -130,7 +130,7 @@ Using Tox (recommended)
 
 
 This creates a virtual environment ``.tox/dev`` (or ``windows-dev``) inside the ``PyBaMM/`` directory.
-It comes ready with PyBaMM and some useful development tools like `flake8 <https://flake8.pycqa.org/en/latest/>`_ and `black <https://black.readthedocs.io/en/stable/>`_.
+It comes ready with PyBaMM and some useful development tools like `pre-commit <https://pre-commit.com/>`_ and `black <https://black.readthedocs.io/en/stable/>`_.
 
 You can now activate the environment with
 
@@ -228,7 +228,6 @@ Doctests, examples, style and coverage
 --------------------------------------
 
 - ``tox -e examples``: Run the example scripts in ``examples/scripts``.
-- ``tox -e flake8``: Check for PEP8 compliance.
 - ``tox -e doctests``: Run doctests.
 - ``tox -e coverage``: Measure current test coverage.
 

run-tests.py

@@ -58,30 +58,6 @@ def run_code_tests(executable=False, folder: str = "unit", interpreter="python")
         sys.exit(ret)
 
 
-def run_flake8():
-    """
-    Runs flake8 in a subprocess, exits if it doesn't finish.
-    """
-    print("Running flake8 ... ")
-    sys.stdout.flush()
-    p = subprocess.Popen(["flake8"], stderr=subprocess.PIPE)
-    try:
-        ret = p.wait()
-    except KeyboardInterrupt:
-        try:
-            p.terminate()
-        except OSError:
-            pass
-        p.wait()
-        print("")
-        sys.exit(1)
-    if ret == 0:
-        print("ok")
-    else:
-        print("FAILED")
-        sys.exit(ret)
-
-
 def run_doc_tests():
     """
     Checks if the documentation can be built, runs any doctests (currently not
@@ -328,17 +304,14 @@ def export_notebook(ipath, opath):
         help="Run unit tests using the `python` interpreter.",
     )
     parser.add_argument(
-        "--nosub",
+        "--all",
         action="store_true",
-        help="Run unit tests without starting a subprocess.",
+        help="Run all tests (unit and integration) using the `python` interpreter.",
     )
-    # Daily tests vs unit tests
     parser.add_argument(
-        "--folder",
-        nargs=1,
-        default=["unit"],
-        choices=["unit", "integration", "all"],
-        help="Which folder to run the tests from.",
+        "--nosub",
+        action="store_true",
+        help="Run unit tests without starting a subprocess.",
     )
     # Notebook tests
     parser.add_argument(
@@ -352,9 +325,11 @@ def export_notebook(ipath, opath):
         metavar=("in", "out"),
         help="Export a Jupyter notebook to a Python file for manual testing.",
     )
-    # Doctests
+    # Flake8 (deprecated)
     parser.add_argument(
-        "--flake8", action="store_true", help="Run flake8 to check for style issues"
+        "--flake8",
+        action="store_true",
+        help="Run flake8 to check for style issues (deprecated, use pre-commit)",
     )
     # Doctests
     parser.add_argument(
@@ -366,7 +341,7 @@ def export_notebook(ipath, opath):
     parser.add_argument(
         "--quick",
         action="store_true",
-        help="Run quick checks (unit tests, flake8, docs)",
+        help="Run quick checks (code tests, docs)",
     )
     # Non-standard Python interpreter name for subprocesses
     parser.add_argument(
@@ -383,23 +358,23 @@ def export_notebook(ipath, opath):
     # Run tests
     has_run = False
     # Unit vs integration
-    folder = args.folder[0]
     interpreter = args.interpreter
     # Unit tests
     if args.integration:
         has_run = True
-        folder = args.folder[1]
-        run_code_tests(True, folder, interpreter)
+        run_code_tests(True, "integration", interpreter)
     if args.unit:
         has_run = True
-        run_code_tests(True, folder, interpreter)
+        run_code_tests(True, "unit", interpreter)
+    if args.all:
+        has_run = True
+        run_code_tests(True, "all", interpreter)
     if args.nosub:
         has_run = True
-        run_code_tests(folder=folder, interpreter=interpreter)
+        run_code_tests(folder="unit", interpreter=interpreter)
     # Flake8
     if args.flake8:
-        has_run = True
-        run_flake8()
+        raise NotImplementedError("flake8 is no longer used. Use pre-commit instead.")
     # Doctests
     if args.doctest:
         has_run = True
@@ -414,8 +389,7 @@ def export_notebook(ipath, opath):
     # Combined test sets
     if args.quick:
         has_run = True
-        run_flake8()
-        run_code_tests(folder, interpreter=interpreter)
+        run_code_tests("all", interpreter=interpreter)
         run_doc_tests()
     # Help
     if not has_run:

setup.py

@@ -216,7 +216,7 @@ def compile_KLU():
             "myst-parser",
         ],  # For doc generation
         "dev": [
-            "flake8>=3",  # For code style checking
+            "pre-commit",  # For code style checking
             "black",  # For code style auto-formatting
         ],
     },

tox.ini

@@ -12,18 +12,18 @@ setenv =
 deps =
      dev-!windows-!mac: cmake
      dev: black
-     dev,doctests: sphinx>=1.5
-     dev,doctests: pydata-sphinx-theme
-     dev,doctests: sphinx_design
-     dev,doctests: sphinx-copybutton
-     dev,doctests: myst-parser
+     doctests: sphinx>=1.5
+     doctests: pydata-sphinx-theme
+     doctests: sphinx_design
+     doctests: sphinx-copybutton
+     doctests: myst-parser
      !windows-!mac: scikits.odes
 
 commands =
      tests-!windows-!mac: sh -c "pybamm_install_jax"  # install jax, jaxlib for ubuntu
-     tests: python run-tests.py --unit --folder all
+     tests: python run-tests.py --all
      unit: python run-tests.py --unit
-     integration: python run-tests.py --unit --folder integration
+     integration: python run-tests.py --integration
      examples: python run-tests.py --examples
      dev-!windows-!mac: sh -c "echo export LD_LIBRARY_PATH={env:LD_LIBRARY_PATH} >> {envbindir}/activate"
      doctests: python run-tests.py --doctest
@@ -62,6 +62,9 @@ deps =
      sphinx>=1.5
      pydata-sphinx-theme
      sphinx-autobuild
+     sphinx_design
+     sphinx-copybutton
+     myst-parser
 changedir = docs
 commands = sphinx-autobuild --open-browser -qT . {envtmpdir}/html