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

Adds PR Template #214

Merged
merged 9 commits into from
Mar 6, 2024
36 changes: 36 additions & 0 deletions .github/PULL_REQUEST_TEMPLATE.md
Original file line number Diff line number Diff line change
@@ -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.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

## Features

- [#185](https://github.com/pybop-team/PyBOP/pull/185) - Adds a pull request template, additional nox sessions `quick` for standard tests + docs, `pre-commit` for pre-commit, `test` to run all standard tests, `doctest` for docs.
- [#215](https://github.com/pybop-team/PyBOP/pull/215) - Adds `release_workflow.md` and updates `release_action.yaml`
- [#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.
Expand Down
39 changes: 29 additions & 10 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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!

Expand Down Expand Up @@ -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

Expand All @@ -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

Expand Down Expand Up @@ -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

Expand Down
5 changes: 4 additions & 1 deletion conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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", "docs: mark test as a doc test")


def pytest_collection_modifyitems(config, items):
Expand All @@ -50,6 +52,7 @@ def pytest_collection_modifyitems(config, items):
"integration": "integration",
"plots": "plots",
"notebooks": "notebooks",
"docs": "docs",
}
selected_markers = [
marker for option, marker in options.items() if config.getoption(option)
Expand All @@ -63,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)
Expand Down
5 changes: 2 additions & 3 deletions docs/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 -----------------------------------------------------
Expand Down
50 changes: 49 additions & 1 deletion noxfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")


Expand All @@ -64,6 +69,49 @@ def notebooks(session):
)


@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", ".[plot,docs,dev]", silent=False)
session.run("pytest", "--docs", "-n", "0")


@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):
"""
Expand Down
48 changes: 48 additions & 0 deletions tests/docs/test_docs.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import sys
import pytest
import shutil
import subprocess
from pathlib import Path


class TestDocs:
"""A class to test the PyBOP documentation."""

@pytest.mark.docs
def test_docs(self):
"""
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(
[
"sphinx-build",
"-j",
"auto",
"-b",
"html",
str(docs_path),
str(build_path),
"--keep-going",
],
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(build_path)
except Exception as e:
print(f"Error deleting built files: {e}")
Loading