Skip to content

Latest commit

 

History

History
160 lines (116 loc) · 4.77 KB

File metadata and controls

160 lines (116 loc) · 4.77 KB

Charm Tech Python style guide

This is the Python code style we're converging on across the projects maintained by the Charm Tech team. For documentation and docstring style (which applies to all languages), see docs.md.

We use Ruff for formatting, and run our code through the Pyright type checker. We try to follow PEP 8, the official Python style guide. However, PEP 8 is fairly low-level, so in addition we've come up with the following style guidelines. See Tooling configuration below for the standardised lint/format/type-check config that enforces much of this.

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.

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:

from ops import CharmBase, PebbleReadyEvent
from subprocess import run

class MyCharm(CharmBase):
    def _pebble_ready(self, event: PebbleReadyEvent):
        run(['echo', 'foo'])

Do:

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]]

Use relative imports inside a package

When writing code inside a package (a directory containing an __init__.py file), use relative imports with a . instead of absolute imports. For example, within the ops package:

Don't:

from ops import charm

Do:

from . import charm

# Or, if you need to avoid adding the public name "charm" to the namespace:

from . import charm as _charm

Avoid nested comprehensions and generator expressions

"Flat is better than nested."

Don't:

units = [unit for app in model.apps for unit in app.units]

for current in (
    current for current in pebble.ServiceStatus if current is not pebble.ServiceStatus.ACTIVE
):
    ...

Do:

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 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:

if status == pebble.ServiceStatus.ACTIVE:
    print('Running')

if status != pebble.ServiceStatus.ACTIVE:
    print('Stopped')

Do:

if status is pebble.ServiceStatus.ACTIVE:
    print('Running')

if status is not pebble.ServiceStatus.ACTIVE:
    print('Stopped')

Tooling configuration

Our repos (operator, jubilant, pytest-jubilant, charmhub-listing-review, charmlibs) have converged on a common lint/format/type-check configuration. The aim is consistency, not lock-step: a project can deviate where it has a good reason, but this is the default.

The decisions:

  • Line length is 99.
  • Single quotes for strings (ruff format quote-style = "single").
  • Type-checking is strict (pyright), with reportPrivateUsage = false (things that are effectively public still need to be private to users) and reportUnnecessaryTypeIgnoreComment = "error".
  • Set target-version / pythonVersion to the project's actual minimum supported Python — don't leave it stale (several repos disagreed with their own requires-python).
  • Coverage runs with branch = true.
  • The agreed ruff rule set is: F, E, W, I001, N, A, CPY, UP, YTT, S, B, SIM, RUF, PERF, D, FA, TC — ignoring TC001TC003 (don't force imports into type-checking blocks), S101 (assert is fine), and D105/D107 (no docstrings required for magic/__init__ methods). Tests additionally drop D and the hard-coded-secret checks (S101/S105/S106).

The ready-to-copy pyproject.toml config itself will be distributed via the shared-template mechanism (see #6); this section records what we standardised on.