From 900812ee44ada4e41f3f50d66f1e364946128f30 Mon Sep 17 00:00:00 2001 From: Ben Hoyt Date: Tue, 8 Oct 2024 15:48:00 +1300 Subject: [PATCH] docs: the very start of a team Python style guide (#1409) This adds the very start of a team Python style guide, with a few items we've discussed recently. The idea is to add as things come up (more than once) in PRs and we feel it's worth adding them to the style guide to avoid having to make a specific decision each time. --- HACKING.md | 11 ++--- STYLE.md | 123 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 127 insertions(+), 7 deletions(-) create mode 100644 STYLE.md diff --git a/HACKING.md b/HACKING.md index a61932c0d..0ba4ffb55 100644 --- a/HACKING.md +++ b/HACKING.md @@ -23,7 +23,7 @@ tox tox -e unit tox -e unit test/test_charm.py -# Format the code using isort and autopep8 +# Format the code using Ruff tox -e fmt # Compile the requirements.txt file for docs @@ -164,6 +164,8 @@ a bunch of charms that use the operator framework. The script can be run locally Changes are proposed as [pull requests on GitHub](https://github.com/canonical/operator/pulls). +For coding style, we follow [PEP 8](https://peps.python.org/pep-0008/) as well as a team [Python style guide](./STYLE.md). + Pull requests should have a short title that follows the [conventional commit style](https://www.conventionalcommits.org/en/) using one of these types: @@ -293,12 +295,7 @@ Static type checking is done using [pyright](https://github.com/microsoft/pyrigh and extends the Python 3.8 type hinting support through the [typing_extensions](https://pypi.org/project/typing-extensions/) package. -Formatting uses [isort](https://pypi.org/project/isort/) and -[autopep8](https://pypi.org/project/autopep8/), with linting also using -[flake8](https://github.com/PyCQA/flake8), including the -[docstrings](https://pypi.org/project/flake8-docstrings/), -[builtins](https://pypi.org/project/flake8-builtins/) and -[pep8-naming](https://pypi.org/project/pep8-naming/) extensions. +Formatting uses [Ruff](https://docs.astral.sh/ruff/). All tool configuration is kept in [project.toml](pyproject.toml). The list of dependencies can be found in the relevant `tox.ini` environment `deps` field. diff --git a/STYLE.md b/STYLE.md new file mode 100644 index 000000000..1a5361be0 --- /dev/null +++ b/STYLE.md @@ -0,0 +1,123 @@ +# Ops Python style guide + +This is the Python style guide we use for the Ops project. It's also the style we're converging on for other projects maintained by the Charm Tech team. + +We use Ruff for formatting, and run our code through the Pyright type checker. We try to follow [PEP 8](https://peps.python.org/pep-0008/), the official Python style guide. However, PEP 8 is fairly low-level, so in addition we've come up with the following style guidelines. + +New code should follow these guidelines, unless there's a good reason not to. Sometimes existing code doesn't follow these, but we're happy for it to be updated to do so (either all at once, or as you change nearby code). + +Of course, this is just a start! We add to this list as things come up in code review; this list reflects our team decisions. + + +**Table of contents:** + +* [Code decisions](#code-decisions) +* [Docs and docstrings](#docs-and-docstrings) + + +## Code decisions + +### Import modules, not objects + +"Namespaces are one honking great idea -- let's do more of those!" + +When reading code, it's significantly easier to tell where a name came from if it is prefixed with the package name. + +An exception is names from `typing` -- type annotations get too verbose if these all have to be prefixed with `typing.`. + +**Don't:** + +```python +from ops import CharmBase, PebbleReadyEvent +from subprocess import run + +class MyCharm(CharmBase): + def _pebble_ready(self, event: PebbleReadyEvent): + run(['echo', 'foo']) +``` + +**Do:** + +```python +import ops +import subprocess + +class MyCharm(ops.CharmBase): + def _pebble_ready(self, event: ops.PebbleReadyEvent): + subprocess.run(['echo', 'foo']) + +# However, "from typing import Foo" is okay to avoid verbosity +from typing import Optional, Tuple +counts: Optional[Tuple[str, int]] +``` + + +### Avoid nested comprehensions and generator expressions + +"Flat is better than nested." + +**Don't:** + +```python +units = [units for app in model.apps for unit in app.units] + +for current in ( + status for status in pebble.ServiceStatus if status is not pebble.ServiceStatus.ACTIVE +): + ... +``` + +**Do:** + +```python +units = [] +for app in model.apps: + for unit in app.units: + units.append(unit) + +for status in pebble.ServiceStatus: + if status is pebble.ServiceStatus.ACTIVE: + continue + ... +``` + + +### Compare enum values by identity + +The [Enum HOWTO](https://docs.python.org/3/howto/enum.html#comparisons) says that "enum values are compared by identity", so we've decided to follow that. Note that this decision applies to regular `enum.Enum` values, not to `enum.IntEnum` or `enum.StrEnum` (the latter is only available from Python 3.11). + +**Don't:** + +```python +if status == pebble.ServiceStatus.ACTIVE: + print('Running') + +if status != pebble.ServiceStatus.ACTIVE: + print('Stopped') +``` + +**Do:** + +```python +if status is pebble.ServiceStatus.ACTIVE: + print('Running') + +if status is not pebble.ServiceStatus.ACTIVE: + print('Stopped') +``` + + +## Docs and docstrings + +### Use British English + +[Canonical's documentation style](https://docs.ubuntu.com/styleguide/en/) uses British spelling, which we try to follow here. For example: "colour" rather than "color", "labelled" rather than "labeled", "serialise" rather than "serialize", and so on. + +It's a bit less clear when we're dealing with code and APIs, as those normally use US English, for example, `pytest.mark.parametrize`, and `color: #fff`. + + +### Spell out abbreviations + +Abbreviations and acronyms in docstrings should usually be spelled out, for example, "for example" rather than "e.g.", "that is" rather than "i.e.", "and so on" rather than "etc", and "unit testing" rather than UT. + +However, it's okay to use acronyms that are very well known in our domain, like HTTP or JSON or RPC.