Style Guide
Syntax
License
Python files the GPLv3 license. A blank line should separate the license from the imports
Example:
# Copyright 2023 The Wazo Authors (see the AUTHORS file)
# SPDX-License-Identifier: GPL-3.0-or-later
import argparse
Spacing
- Lines should not go further than 80 to 100 characters.
- In Python, indentation blocks use 4 spaces
- Imports should be sorted alphabetically
- Separate module imports and
from
imports with a blank line
Example:
import argparse
import datetime
import os
import re
import shutil
import tempfile
from io import StringIO
from urllib.parse import urlencode
General Style Rules
To try and maintain a clean and consistent code base we use black
, which is a tool that enforces a
slightly stricter subset of Python's PEP8 and flake8
. There
is a good explanation of the rules and reasons on
its website. You
can run linters in all projects via tox
with tox -e linters
to check if your code is correctly
formatted. The CI will run this automatically too and fail if the code isn't conform.
In newer projects we also have pre-commit
to run and apply the fixes
automatically in some cases as well as mypy
for static
type checking (see type checking below. Pre-commit can be installed via pip
(pip install pre-commit
) and either run manually via pre-commit run --all-files
or automatically
as a hook (just run pre-commit install
in the repo and it will run automatically in future before
each commit). For backwards compatibility it can also be run via tox -e linters
.
Strings
Avoid using the +
operator for concatenating strings. Use string interpolation ("f-strings")
instead.
Bad Example:
phone_interface = 'SIP' + '/' + username + '-' + password
Good Example:
phone_interface = f'SIP/{username}-{password}'
Comments
Redundant comments should be avoided. Instead, effort should be put on making the code clearer.
Bad Example:
# Add the meeting to the calendar only if it was created on a week day
# (monday to friday)
if meeting.day > 0 and meeting.day < 7:
calendar.add(meeting)
Good Example:
def created_on_week_day(meeting: Meeting) -> bool:
return 0 < meeting.day < 7
if created_on_week_day(meeting):
calendar.add(meeting)
Use functions for clarity
Consider refactoring your statement into a function if it becomes too long, or the meaning isn't clear.
Bad Example:
if price * tax - bonus / reduction + fee < money:
product.pay(money)
Good Example:
def calculate_price(price: float, tax: float, bonus: float, reduction: float, fee: float) -> float:
return price * tax - bonus / reduction + fee
final_price = calculate_price(price, tax, bonus, reduction, fee)
if final_price < money:
product.pay(money)
Naming
- Class names are in
PascalCase
(Upper Camel Case) - File, method and variable names are in
lower_snake_case
- "constants" are in
UPPER_CASE
Conventions for functions prefixed by find
:
- Return None when nothing is found
- Return an object when a single entity is found
- Return the first element when multiple entities are found
Example:
def find_by_username(username: str) -> User | None:
users = [user1, user2, user3]
user_search = [user for user in users if user.username == username]
if len(user_search) == 0:
return None
return user_search[0]
Conventions for functions prefixed by get
:
- Raise an Exception when nothing is found
- Return an object when a single entity is found
- Return the first element when multiple entities are found
Example:
def get_user(user_id: str) -> User:
users = [user1, user2, user3]
user_search = [user for user in users if user.userid == user_id]
if len(user_search) == 0:
raise UserNotFoundError(user_id)
return user_search[0]
Conventions for functions prefixed by find_all
:
- Return an empty list when nothing is found
- Return a list of objects when multiple entities are found
Example:
def find_all_users_by_username(username: str) -> list[User]:
users = [user1, user2, user3]
user_search = [user for user in users if user.username == username]
return user_search
Magic numbers
Magic numbers should be avoided. Arbitrary values should be assigned to variables with a clear name.
Bad example:
class TestRanking(TestCase):
def test_ranking(self) -> None:
rank = Rank(1, 2, 3)
self.assertEquals(rank.position, 1)
self.assertEquals(rank.grade, 2)
self.assertEquals(rank.session, 3)
Good example:
class TestRanking(TestCase):
def test_ranking(self) -> None:
position = 1
grade = 2
session = 3
rank = Rank(position, grade, session)
self.assertEquals(rank.position, position)
self.assertEquals(rank.grade, grade)
self.assertEquals(rank.session, session)
Assertions
Using assert
in production code is accepted as long as it is not used for validation of
untrusted input. It must only be used to document critical expectations, which if violated would
result in unexpected behavior in the code following the assert
. It can also be used in combination
with type annotations to provide information to a static type checker for
type narrowing.
Bad example:
def get_contacts(self, contact_source_id: str) -> PhonebookSourceInfo:
try:
assert database.contact_source_exists(contact_source_id)
except AssertionError:
raise LookupError('Contact source id "{contact_source_id}" does not exist')
...
In this case assert
is used to validate an expected error condition which must be accounted for
and handled. Instead, simply use a conditional statement(e.g.
if not database.contact_source_exists(contact_source_id): ...
) to validate those kinds of
conditions and act appropriately.
Good example:
def get_contacts(self, contact_source_id: str) -> PhonebookSourceInfo:
contact_source = database.get_contact_source(contact_source_id)
if contact_source['type'] == 'phonebook':
return get_phonebook_contacts(contact_source)
def get_phonebook_contacts(self, source_data: dict):
assert 'phonebook_uuid' in source_data
phonebook_key = PhonebookKey(uuid=source_data['phonebook_uuid'])
...
Here, the assertion indicates that the code of get_phonebook_contacts
expects its input to have a
specific key, and is not designed to be used with arbitrary arguments that do not contain this key,
and that there is no intention to handle such a case. An AssertionError
resulting from a violation
of that expectation would signal, hopefully during testing, that the function is not used properly
by the surrounding code. This is similar to the use of static type annotations in informing
developers on the intended usage of interfaces and guaranteeing correctness of a part of the
implementation by detecting potential bugs that could otherwise remain silent.
Tests
Place tests along side the code
Tests for a package are placed in their own folder named tests
inside the package.
Example:
├── package1
│ ├── __init__.py
│ ├── mod1.py
│ └── tests/
│ ├── __init__.py
│ └── test_mod1.py
├── package2/
│ ├── __init__.py
│ ├── mod9.py
│ └── tests/
│ ├── __init__.py
│ └── test_mod9.py
Short and clear tests
Unit tests should be short, clear and concise in order to make the test easy to understand. A unit test is separated into 3 sections:
- Preconditions / Preparations
- Thing to test
- Assertions
Sections are separated by a blank line. Sections that become too big should be split into smaller functions.
Example:
class UserTestCase(TestCase):
def test_full_name(self) -> None:
user = User(firstname='Bob', lastname='Marley')
expected = 'Bob Marley'
fullname = user.full_name()
self.assertEquals(expected, fullname)
def _prepare_expected_user(self, first_name: str, last_name: str, number: str) -> User:
user = User()
user.first_name = first_name
user.last_name = last_name
user.number = number
return user
def _assert_users_are_equal(self, expected_user: User, actual_user: User) -> None:
self.assertEquals(expected_user.first_name, actual_user.first_name)
self.assertEquals(expected_user.last_name, actual_user.last_name)
self.assertEquals(expected_user.number, actual_user.number)
def test_create_user(self):
expected = self._prepare_expected_user('Bob', 'Marley', '4185551234')
user = create_user('Bob', 'Marley', '4185551234')
self._assert_users_are_equal(expected, user)
Exceptions
Don't use exceptions for flow control
Exceptions should not be used for flow control. Raise exceptions only for edge cases, or when something that isn't usually expected happens.
Bad Example:
def is_user_available(user: User) -> bool:
if user.available():
return True
raise Exception("User isn't available")
try:
is_user_available(user)
except Exception:
disable_user(user)
Good Example:
def is_user_available(user: User) -> bool:
return user.available()
if not is_user_available(user):
disable_user(user)
Avoid throwing a generic Exception
Use one of Python's built-in Exceptions, or create your own custom Exception class. This helps ensure that the cause of the Exception is clear and allows you to safely catch expected exceptions and not accidentally silence unexpected ones. A list of exceptions is available on the Python documentation website.
Bad Example:
def get_user(user_id: str) -> User:
user = session.query(User).get(user_id)
if not user:
raise Exception("User not found")
Good Example:
class UserNotFoundError(LookupError):
def __init__(self, user_id: str) -> None:
super().__init__(f"User with id {user_id} not found")
def get_user(user_id: str) -> User:
user = session.query(User).get(user_id)
if not user:
raise UserNotFoundError(userid)
return user
Always specify an Exception in except:
blocks
Never use except:
without specifying any exception type. The reason is that it will also catch
important exceptions, such as KeyboardInterrupt
and OutOfMemory
exceptions, making your program
unstoppable or continuously failing, instead of stopping when wanted. This also avoids accidentally
catching unexpected issues which then fail silently.
Bad Example:
try:
get_user(user_id)
except:
logger.exception("There was an error")
Good Example:
try:
get_user(user_id)
except UserNotFoundError as e:
logger.error(e.message)
raise
Type Hinting
When possible, code should include type hints to help avoid ambiguity, help with debugging and, allow for static type checking. For some common use cases and more examples, please see Type hinting examples.
Clarity
Type hinting allows one to clearly identify what a function receives and what it returns. The more precise you can get with your typing the easier the code will be to understand.
Bad Example:
def get_user(user_id):
user = find_user(user_id)
if not user:
raise UserNotFound(user_id)
return user
In this example, it is unclear what we are dealing with. Is the user_id
a str
, an int
, a
UUID
? What is the user
we return? A User
object, a dict (if so what keys?), a name?
Good example:
from typing import TypedDict, Union, TYPE_CHECKING
class UserData(TypedDict):
first_name: str
last_name: str
email: str | None
def get_user(user_id: str) -> UserData:
user = find_user(user_id)
if not user:
raise UserNotFound(user_id)
return user
Running the type checker (mypy)
We use mypy
to do type checking, and it is run via
pre-commit
. It can either be run automatically as a git pre-commit hook
(after pre-commit install
in your repo), manually with pre-commit run --all-files
or via tox
with tox -e linters
. It is configured in the pyproject.toml
file.
Laziness
Any file that contains type annotations should, ideally, include
from __future__ import annotations
at the top. This ensures:
- We don't waste effort evaluating types at runtime
- We can use features from later version of Python without errors at runtime
- You can reference classes before they are defined
Always including it will avoid accidentally forgetting it when adding new types too. See more about lazy type annotations here