From 6019c9ad74818c62df6b214d490b4f0f764717ae Mon Sep 17 00:00:00 2001 From: Brady Planden Date: Thu, 22 Feb 2024 20:16:07 +0000 Subject: [PATCH 1/6] Add PR template, updt. docs conf.py path definition, add doc tests, add noxfile sessions --- .github/PULL_REQUEST_TEMPLATE.md | 36 ++++++++++++++++++++++++++ docs/conf.py | 5 ++-- noxfile.py | 43 +++++++++++++++++++++++++++++++ tests/docs/test_docs.py | 44 ++++++++++++++++++++++++++++++++ 4 files changed, 125 insertions(+), 3 deletions(-) create mode 100644 .github/PULL_REQUEST_TEMPLATE.md create mode 100644 tests/docs/test_docs.py diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md new file mode 100644 index 000000000..bb84b43ed --- /dev/null +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -0,0 +1,36 @@ +# Description + +Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change. + +## Issue reference +Fixes # (issue-number) + +## Review +Before you mark your PR as ready for review, please ensure that you've considered the following: +- Updated the [CHANGELOG.md](https://github.com/pybop-team/PyBOP/blob/develop/CHANGELOG.md) in reverse chronological order (newest at the top) with a concise description of the changes, including the PR number. +- Noted any breaking changes, including details on how it might impact existing functionality. + +## Type of change +- [ ] New Feature: A non-breaking change that adds new functionality. +- [ ] Optimization: A code change that improves performance. +- [ ] Bug Fix: A non-breaking change that addresses an issue. +- [ ] Documentation: Updates to documentation or new documentation for new features. +- [ ] Refactoring: Non-functional changes that improve the codebase. +- [ ] Style: Non-functional changes related to code style (formatting, naming, etc). +- [ ] Testing: Additional tests to improve coverage or confirm functionality. +- [ ] Other: (Insert description of change) + +# Key checklist: + +- [ ] No style issues: `$ pre-commit run` (or `$ nox -s pre-commit`) (see [CONTRIBUTING.md](https://github.com/pybop-team/PyBOP/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 unit tests pass: `$ nox -s tests` +- [ ] The documentation builds: `$ nox -s docs` + +You can run integration tests, unit tests, and doctests together at once, using `$ nox -s quick`. + +## Further checks: +- [ ] Code is well-commented, especially in complex or unclear areas. +- [ ] Added tests that prove my fix is effective or that my feature works. +- [ ] Checked that coverage remains or improves, and added tests if necessary to maintain or increase coverage. + +Thank you for contributing to our project! Your efforts help us to deliver great software. diff --git a/docs/conf.py b/docs/conf.py index ae94c5adf..cfc37a90f 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -4,10 +4,9 @@ # -- Path setup -------------------------------------------------------------- import os import sys +from pathlib import Path -root_path = os.path.abspath(os.path.join(os.path.dirname(__file__), "..")) -sys.path.insert(0, root_path) - +sys.path.append(str(Path(".").resolve())) from pybop._version import __version__ # noqa: E402 # -- Project information ----------------------------------------------------- diff --git a/noxfile.py b/noxfile.py index e88df2604..5c614eb99 100644 --- a/noxfile.py +++ b/noxfile.py @@ -42,6 +42,49 @@ def notebooks(session): session.run("pytest", "--nbmake", "--examples", "examples/", external=True) +@nox.session(name="tests") +def run_tests(session): + """Run all the tests.""" + session.install("-e", ".[all,dev]", silent=False) + if PYBOP_SCHEDULED: + session.run("pip", "install", f"pybamm=={PYBAMM_VERSION}", silent=False) + session.run( + "pytest", "--unit", "--integration", "--nbmake", "--examples", "-n", "auto" + ) + + +@nox.session(name="doctest") +def run_doc_tests(session): + """ + Checks if the documentation can be built, runs any doctests (currently not + used). + """ + session.install("-e", ".[all,docs,dev]", silent=False) + session.run("pytest", "--docs") + + +@nox.session(name="pre-commit") +def lint(session): + """ + Check all files against the defined pre-commit hooks. + + Credit: PyBaMM Team + """ + session.install("pre-commit", silent=False) + session.run("pre-commit", "run", "--all-files") + + +@nox.session(name="quick", reuse_venv=True) +def run_quick(session): + """ + Run integration tests, unit tests, and doctests sequentially + + Credit: PyBaMM Team + """ + run_tests(session) + run_doc_tests(session) + + @nox.session def docs(session): """ diff --git a/tests/docs/test_docs.py b/tests/docs/test_docs.py new file mode 100644 index 000000000..02b025072 --- /dev/null +++ b/tests/docs/test_docs.py @@ -0,0 +1,44 @@ +import sys +import pytest +import shutil +import subprocess + + +class TestDocs: + """ + A class to test the pybop documentation + """ + + @pytest.mark.docs + def test_docs(self): + """ + Checks if the documentation can be built, runs any doctests (currently not + used). + + Credit: PyBaMM Team + """ + print("Checking if docs can be built.") + try: + subprocess.run( + [ + "sphinx-build", + "-j", + "auto", + "-b", + "html", + "--keep-going", + ".", + "_build/html", + ], + check=True, + ) + except subprocess.CalledProcessError as e: + print(f"FAILED with exit code {e.returncode}") + sys.exit(e.returncode) + finally: + # Regardless of whether the doctests pass or fail, attempt to remove the built files. + print("Deleting built files.") + try: + shutil.rmtree("docs/_build/html/.doctrees/") + except Exception as e: + print(f"Error deleting built files: {e}") From ec564efb5d3ad22acc86bcdc8ef45ca31eca2be3 Mon Sep 17 00:00:00 2001 From: Brady Planden Date: Thu, 22 Feb 2024 20:23:32 +0000 Subject: [PATCH 2/6] Updt. conftest for --docs arg --- conftest.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/conftest.py b/conftest.py index 3641cbd11..25cc4e34d 100644 --- a/conftest.py +++ b/conftest.py @@ -25,6 +25,7 @@ def pytest_addoption(parser): parser.addoption( "--notebooks", action="store_true", default=False, help="run notebook tests" ) + parser.addoption("--docs", action="store_true", default=False, help="run doc tests") def pytest_terminal_summary(terminalreporter, exitstatus, config): @@ -41,6 +42,7 @@ def pytest_configure(config): config.addinivalue_line("markers", "examples: mark test as an example") config.addinivalue_line("markers", "plots: mark test as a plot test") config.addinivalue_line("markers", "notebook: mark test as a notebook test") + config.addinivalue_line("markers", "doc: mark test as a doc test") def pytest_collection_modifyitems(config, items): @@ -50,6 +52,7 @@ def pytest_collection_modifyitems(config, items): "integration": "integration", "plots": "plots", "notebooks": "notebooks", + "docs": "doc", } selected_markers = [ marker for option, marker in options.items() if config.getoption(option) From 3a8800fd87308fde0a7abdd161b454402ba65eea Mon Sep 17 00:00:00 2001 From: Brady Planden Date: Fri, 23 Feb 2024 12:50:56 +0000 Subject: [PATCH 3/6] Updt. Changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 283dd7e6a..391e56aa0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Features +- [#185](https://github.com/pybop-team/PyBOP/pull/185) - Adds pull request template. - [#204](https://github.com/pybop-team/PyBOP/pull/204) - Splits integration, unit, examples, plots tests, update workflows. Adds pytest `--examples`, `--integration`, `--plots` args. Adds tests for coverage after removal of examples. Adds examples and integrations nox sessions. Adds `pybop.RMSE._evaluateS1()` method - [#206](https://github.com/pybop-team/PyBOP/pull/206) - Adds Python 3.12 support with corresponding github actions changes. - [#18](https://github.com/pybop-team/PyBOP/pull/18) - Adds geometric parameter fitting capability, via `model.rebuild()` with `model.rebuild_parameters`. From f447cb43fa397ad63633896f1aaa7db70c72a9ce Mon Sep 17 00:00:00 2001 From: Brady Planden Date: Tue, 5 Mar 2024 21:05:02 +0000 Subject: [PATCH 4/6] fix nox doctest session, pytest docs update --- conftest.py | 2 +- noxfile.py | 2 +- tests/docs/test_docs.py | 20 ++++++++++++-------- 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/conftest.py b/conftest.py index 25cc4e34d..23dc3da3b 100644 --- a/conftest.py +++ b/conftest.py @@ -52,7 +52,7 @@ def pytest_collection_modifyitems(config, items): "integration": "integration", "plots": "plots", "notebooks": "notebooks", - "docs": "doc", + "docs": "docs", } selected_markers = [ marker for option, marker in options.items() if config.getoption(option) diff --git a/noxfile.py b/noxfile.py index 134fc21c3..341f245c1 100644 --- a/noxfile.py +++ b/noxfile.py @@ -82,7 +82,7 @@ def run_doc_tests(session): used). """ session.install("-e", ".[all,docs,dev]", silent=False) - session.run("pytest", "--docs") + session.run("pytest", "--docs", "-n", "0") @nox.session(name="pre-commit") diff --git a/tests/docs/test_docs.py b/tests/docs/test_docs.py index 02b025072..8dd52687a 100644 --- a/tests/docs/test_docs.py +++ b/tests/docs/test_docs.py @@ -2,22 +2,23 @@ import pytest import shutil import subprocess +from pathlib import Path class TestDocs: - """ - A class to test the pybop documentation - """ + """A class to test the PyBOP documentation.""" @pytest.mark.docs def test_docs(self): """ - Checks if the documentation can be built, runs any doctests (currently not - used). + Check if the documentation can be built and run any doctests (currently not used). Credit: PyBaMM Team """ print("Checking if docs can be built.") + docs_path = Path("docs") + build_path = docs_path / "_build" / "html" + try: subprocess.run( [ @@ -26,19 +27,22 @@ def test_docs(self): "auto", "-b", "html", + str(docs_path), + str(build_path), "--keep-going", - ".", - "_build/html", ], check=True, + capture_output=True, ) except subprocess.CalledProcessError as e: print(f"FAILED with exit code {e.returncode}") + print(f"stdout: {e.stdout.decode()}") + print(f"stderr: {e.stderr.decode()}") sys.exit(e.returncode) finally: # Regardless of whether the doctests pass or fail, attempt to remove the built files. print("Deleting built files.") try: - shutil.rmtree("docs/_build/html/.doctrees/") + shutil.rmtree(build_path) except Exception as e: print(f"Error deleting built files: {e}") From c3c15a41bc69634a1141e8b7e40a845ee0113f25 Mon Sep 17 00:00:00 2001 From: Brady Planden Date: Tue, 5 Mar 2024 22:13:27 +0000 Subject: [PATCH 5/6] add plots session, update nox session installs --- conftest.py | 4 ++-- noxfile.py | 9 +++++++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/conftest.py b/conftest.py index 23dc3da3b..fc14a21e0 100644 --- a/conftest.py +++ b/conftest.py @@ -42,7 +42,7 @@ def pytest_configure(config): config.addinivalue_line("markers", "examples: mark test as an example") config.addinivalue_line("markers", "plots: mark test as a plot test") config.addinivalue_line("markers", "notebook: mark test as a notebook test") - config.addinivalue_line("markers", "doc: mark test as a doc test") + config.addinivalue_line("markers", "docs: mark test as a doc test") def pytest_collection_modifyitems(config, items): @@ -66,7 +66,7 @@ def pytest_collection_modifyitems(config, items): # If no options were passed, skip all tests if not selected_markers: skip_all = pytest.mark.skip( - reason="Need at least one of --unit, --examples, --integration, or --plots option to run" + reason="Need at least one of --unit, --examples, --integration, --docs, or --plots option to run" ) for item in items: item.add_marker(skip_all) diff --git a/noxfile.py b/noxfile.py index 341f245c1..e946d8216 100644 --- a/noxfile.py +++ b/noxfile.py @@ -37,10 +37,15 @@ def coverage(session): ) +@nox.session +def plots(session): + session.install("-e", ".[plot,dev]", silent=False) + session.run("pytest", "--plots", "-n", "0") + + @nox.session def integration(session): session.install("-e", ".[all,dev]", silent=False) - session.install("pytest", "pytest-mock") session.run("pytest", "--integration") @@ -81,7 +86,7 @@ def run_doc_tests(session): Checks if the documentation can be built, runs any doctests (currently not used). """ - session.install("-e", ".[all,docs,dev]", silent=False) + session.install("-e", ".[plot,docs,dev]", silent=False) session.run("pytest", "--docs", "-n", "0") From 02ee2e201b5be4be7e7f2fbd55dbcba46dd85eca Mon Sep 17 00:00:00 2001 From: Brady Planden Date: Wed, 6 Mar 2024 12:53:35 +0000 Subject: [PATCH 6/6] update contributing.md --- CONTRIBUTING.md | 39 +++++++++++++++++++++++++++++---------- 1 file changed, 29 insertions(+), 10 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 6d6f8aaa3..0f3a7f07d 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -27,6 +27,12 @@ Before you commit any code, please perform the following checks using [Nox](http `PyBOP` uses a set of `pre-commit` hooks and the `pre-commit` bot to format and prettify the codebase. The hooks can be installed locally using - +```bash +nox -s pre-commit +``` + +alternatively, without nox: + ```bash pip install pre-commit pre-commit install @@ -50,7 +56,7 @@ We use [Git](https://en.wikipedia.org/wiki/Git) and [GitHub](https://en.wikipedi 2. Create a [branch](https://help.github.com/articles/creating-and-deleting-branches-within-your-repository/) of this repo (ideally on your own [fork](https://help.github.com/articles/fork-a-repo/)), where all changes will be made 3. Download the source code onto your local system, by [cloning](https://help.github.com/articles/cloning-a-repository/) the repository (or your fork of the repository). 4. [Install](#developer-installation) PyBOP with the developer options. -5. [Test](#testing) if your installation worked: `$ pytest --unit -v`. +5. [Test](#testing) if your installation worked: `nox -s unit` or `$ pytest --unit -v`. You now have everything you need to start making changes! @@ -112,21 +118,21 @@ On the other hand... We _do_ want to compare several tools, to generate document Only 'core pybop' is installed by default. The others have to be specified explicitly when running the installation command. -### Matplotlib +### Plotly -We use Matplotlib in PyBOP, but with two caveats: +We use Plotly in PyBOP, but with two caveats: -First, Matplotlib should only be used in plotting methods, and these should _never_ be called by other PyBOP methods. So users who don't like Matplotlib will not be forced to use it in any way. Use in notebooks is OK and encouraged. +First, Plotly should only be used in plotting methods, and these should _never_ be called by other PyBOP methods. So users who don't like Plotly will not be forced to use it in any way. Use in notebooks is OK and encouraged. -Second, Matplotlib should never be imported at the module level, but always inside methods. For example: +Second, Plotly should never be imported at the module level, but always inside methods. For example: -``` +```python def plot_great_things(self, x, y, z): - import matplotlib.pyplot as pl + go = pybop.PlotlyManager().go ... ``` -This allows people to (1) use PyBOP without ever importing Matplotlib and (2) configure Matplotlib's back-end in their scripts, which _must_ be done before e.g. `pyplot` is first imported. +This allows people to (1) use PyBOP without ever importing Plotly and (2) configure Plotly's settings in their scripts, which _must_ be done before e.g. `graph_objects` is first imported. ### Building documentation @@ -136,7 +142,11 @@ We use [Sphinx](http://www.sphinx-doc.org/en/stable/) to build our documentation nox -s docs ``` -This will build the docs using sphinx-autobuild and render them in your browser. +This will build the docs using sphinx-autobuild and render them in your browser. Likewise, to test the docs build, the following nox session is available: + +```bash +nox -s doctests +``` ## Testing @@ -165,7 +175,16 @@ And for individual tests, ```bash pytest tests/unit/path/to/test.py::TestClass:test_name --unit -v ``` -where `--unit` is a flag to run only unit tests and `-v` is a flag to display verbose output. +where `--unit` is a flag to run only unit tests and `-v` is a flag to display verbose output. Furthermore, to run all the standard tests, type + +```bash +nox -s tests +``` +Additionally, to run the standard and docs tests, type + +```bash +nox -s quick +``` ### Writing tests