Note: This style guide is for esoteric, project-specific conventions that are not automatically enforced by tools like Ruff or Pyright. It should be kept minimal and focused, and not attempt to duplicate or override existing linting/type-checking policies.
- 1. Prefer
@dataclassfor Class definitions - 2. Almost never use dict.get
- 3. On tests, prefer passing fixture name instead of file bytes
- 4. Use simple test values, not pseudo-realistic ones
- 5. Place private methods/functions before the methods/functions that use them
- 6. Configuration and service settings
- 7. Almost never test private methods/functions
- 8. Use
Structfor pure DTOs,@dataclassfor classes with behavior - 9. Use
create_autospecfor mocking in tests - 10. Almost never use globals
- 11. Prefer operators over mutating method calls
- 12. Almost never use nested functions or classes
- 13. Derive names from the class name
We acknowledge that this is a slight abuse of what dataclass was originally intended for (pure data containers), but in practice, the benefits — reduced boilerplate, clear structure, and ease of use — outweigh the downsides.
from dataclasses import dataclass
@dataclass
class Controller:
_service1: Service1
_service2: Service2
# Use positional args when using this pattern to avoid doing _thing=thing
controller = Controller(service1, service2)If an attribute needs to be set after instantiation, use field(init=False) and use the __post_init__ method to set it.
Dictionary .get method should only ever be used when a field in a dictionary is optional and you want to provide a default value.
For example, if some API returns a dictionary with an optional field then dict.get can be used to elegantly handle the case where the field is not present.
A slightly contrived example would be, maybe an API returns a person object, if they don't have a middle name the api doesn't return this field,
then we can use dict.get to provide a default value of None:
middle_name = person.get("middleName")But if we expect every person to have a first name, then we must use.
first_name = person["firstName"]If the field is required, and the code block can't continue without it, then simply allow a KeyError to be raised.
This makes it clear that the code expects the key to be present, and if it is not, it is a bug that should be fixed.
And, in this case, allowing a default and then raising an error does not provide us with any valable additional information about the error and
actually may hinder us by swallowing the stack trace and making it harder to debug.
For example
Good
value = my_dict["key"]Bad
value = my_dict.get("key")
if value is None:
raise KeyError("Key 'key' is required in dict")When a test fails with a bytes parameter, using actual file bytes, the resulting pytest logs become hard to read. Instead, pass the name of the fixture itself then load the fixture value inside the test function.
For example
Good
def test_example(fixture_name: str, request: pytest.FixtureRequest):
# Instead of using file_bytes directly, use the fixture name
file_bytes = request.getfixturevalue(fixture_name)
#...
assert file_bytes == b"expected bytes"Bad
def test_example(file_bytes: bytes):
# Using file_bytes directly
assert file_bytes == b"expected bytes"In tests, use simple, obvious values instead of pseudo-realistic ones. This makes tests more readable and maintainable, while pseudo-realistic values add zero benefit.
Good
create_user("first-name", "last-name")
create_user.assert_called_with("first-name", "last-name")Bad
first_name = "John"
last_name = "Doe"
create_user(first_name, last_name)
create_user.assert_called_with(first_name, last_name)The simple approach is clearer and eliminates unnecessary variables that don't contribute to the test's purpose.
Also, only provide minimal values that are required for the test to pass or for the interface of the function/method.
Given the following function:
def create_token(token_id: str): ...Good
def test_create_token():
create_token("token-id")Bad
def test_create_token():
create_token("00000000-0000-0000-0000-000000000000")There is no added value in using a pseudo-realistic value like a UUID when a simple string suffices, even if in normal operation this value would be a UUID (unless that function does some validation that forces it to be a UUID, but in this case, the type hint should then be a UUID and the test should then also pass a UUID).
As a general convention, use the kebab-case version of the variable name as the test value. For example, first_name becomes "first-name", last_name becomes "last-name", token_id becomes "token-id", and so on. This keeps test values predictable and trivially derivable from the variable they represent.
Also, prefer inlining these simple test values directly at the call site rather than extracting them into variables or fixtures. A literal like "user-id" is more readable inline than a user_id fixture or constant — there is no shared construction cost or duplication being eliminated, just unnecessary indirection. Reserve fixtures for values that are non-trivial to construct or genuinely benefit from being shared (see Section 10).
Private methods and functions (those prefixed with _) should be defined before the public methods that call them. This improves code readability by following a logical flow where dependencies are defined before their usage.
Note: This differs from languages like Java where private methods are typically placed after public methods. PEP 8 doesn't specify ordering for private vs public methods, so this is a project-specific convention for Python development.
Good
class DocumentProcessor:
def _validate_document(self, doc: bytes) -> bool:
# Private validation logic
return True
def _extract_metadata(self, doc: bytes) -> dict:
# Private extraction logic
return {}
def process_document(self, doc: bytes) -> dict:
if not self._validate_document(doc):
raise ValueError("Invalid document")
return self._extract_metadata(doc)Bad
class DocumentProcessor:
def process_document(self, doc: bytes) -> dict:
if not self._validate_document(doc):
raise ValueError("Invalid document")
return self._extract_metadata(doc)
def _validate_document(self, doc: bytes) -> bool:
# Private validation logic
return True
def _extract_metadata(self, doc: bytes) -> dict:
# Private extraction logic
return {}Configuration should be decoupled from service settings. This separation provides several key benefits:
- Testability: Services can be tested in isolation with explicit settings instances, without requiring a full configuration system or environment variables
- Flexibility: Services can have sensible hardcoded defaults while still allowing environment-specific overrides when needed
- Clarity: The distinction between service-level behavior and environment-specific configuration becomes explicit
- Maintainability: Changes to service defaults don't require touching configuration files, and vice versa
Settings should only be pulled from the central configuration system if and when those values need to vary between environments (local-dev, dev, prod).
For all other settings, it is perfectly acceptable—and preferred—to define sensible defaults directly in the service's settings class. These defaults can be overridden from configuration if the need arises in the future.
Good
@dataclass
class MyServiceSettings:
"""Settings for MyService"""
# Environment-specific: varies between local/dev/prod
max_file_size_mb: float
# Service-specific: sensible default, same across all environments
chunk_size: int = 1000
chunk_overlap: int = 100
temperature: float = 0.0class Settings(BaseModel):
my_service: MyServiceSettings
# ... other settings
typed_settings = Settings(
my_service=MyServiceSettings(
max_file_size_mb=_get_float_setting("my_service.max_file_size_mb"),
# chunk_size, chunk_overlap, temperature use their defaults
),
)Bad
# Putting everything in config when most values never change
@dataclass
class MyServiceSettings:
max_file_size_mb: float
chunk_size: int # Same in all environments
chunk_overlap: int # Same in all environments
temperature: float # Same in all environments
# api/config/settings.yaml - unnecessary duplication across environments
default:
my_service:
max_file_size_mb: 100
chunk_size: 1000
chunk_overlap: 100
temperature: 0.0
dev:
my_service:
max_file_size_mb: 250
chunk_size: 1000 # Duplicated
chunk_overlap: 100 # Duplicated
temperature: 0.0 # DuplicatedThis approach keeps configuration files focused on what actually varies between environments, while keeping service logic and its sensible defaults colocated in the service code.
Private methods and functions (prefixed with _) are implementation details. Testing them directly couples tests to internals, making refactoring harder and tests more fragile.
Instead, test the public interface. If a private method has complex logic worth testing, it is a signal it should be extracted into its own public class or function.
We use dependency injection throughout the codebase, which makes this straightforward. Dependencies are injected via the constructor and replaced with mocks in tests. Use mock assertions (e.g. assert_called_once_with) to verify a component interacts with its dependencies correctly, without reaching into private implementation details.
Good — inject the mock handler as a fixture, assert on its interactions:
@pytest.fixture(name="handler_mock")
def _handler_mock(mocker: MockerFixture) -> MagicMock:
return mocker.create_autospec(Handler)
@pytest.fixture(name="service")
def _service(handler_mock: MagicMock) -> Service:
return Service(handler_mock)
def test_service_calls_handler_with_correct_args(
service: Service,
handler_mock: MagicMock,
):
service.process("input")
handler_mock.handle.assert_called_once_with("input")Bad — accessing the private dependency directly instead of using the injected mock:
def test_service_calls_handler_with_correct_args(service: Service):
service.process("input")
service._handler.handle.assert_called_once_with("input") # Accessing internalsWhen a type is a pure data container — a DTO that just carries fields between layers — use a msgspec.Struct. This codebase is msgspec-first, so a Struct is the native vocabulary; it's also slotted (no per-instance __dict__) and constructs faster and uses less memory than @dataclass(slots=True), which is what made slotted dataclasses the right DTO in non-msgspec projects. Use a Struct whether or not the data ever crosses the wire — internal resolved contracts are Structs too.
from msgspec import Struct
class ParsedDocument(Struct):
document_id: str
page_count: int
text: strWhen a class carries behavior or dependencies rather than just data — a controller or service holding injected collaborators — keep the @dataclass approach from §1 (it composes with field(init=False) / __post_init__, which Structs don't):
from dataclasses import dataclass
@dataclass
class Thing:
_service: ServiceRule of thumb: just fields → Struct; fields plus injected dependencies or behavior → @dataclass.
Always use create_autospec(Thing, spec_set=True, instance=True) when creating mocks, rather than MagicMock() or mocker.MagicMock().
spec_set=True: RaisesAttributeErrorif you access or set an attribute that doesn't exist on the real class — catches typos in attribute/method names at test time rather than silently passing.instance=True: Creates a mock that behaves like an instance of the class, not the class itself (correctisinstancechecks, correct method signatures).- Auto-specced methods: All method mocks automatically enforce the real method's signature, so calls with wrong arguments fail immediately.
@pytest.fixture(name="handler_mock")
def _handler_mock() -> MagicMock:
return create_autospec(Handler, spec_set=True, instance=True)Bad — MagicMock() silently accepts any attribute or call signature:
@pytest.fixture(name="handler_mock")
def _handler_mock() -> MagicMock:
return MagicMock() # typos in method names go undetectedModule-level globals (constants, configuration values, or shared state defined outside of a class) are mostly a design smell. They make code harder to test, harder to reason about, and harder to override in different contexts. Prefer encapsulating these values as class attributes (or dependency-injected settings), so that they live alongside the code that uses them and can be substituted in tests or different runtime contexts.
Exception — infrastructure singletons. Stateless, process-wide infrastructure objects are fine at module level: a rich Console(), a logger = logging.getLogger(__name__), and similar. These are write-only sinks with no behavior worth substituting per-instance, and threading them through every constructor adds noise without improving testability.
# Fine at module level
console = Console()
logger = logging.getLogger(__name__)The rule targets data and configuration globals (URLs, lookup tables, default values, mutable state) — not logging/output plumbing.
Good — values are encapsulated as class attributes:
class Client:
_http_client: niquests.Session
_base_url: str = "http://url/api"Bad — values leak into module scope as globals:
_BASE_URL: str = "http://url/api"
class Client:
_http_client: niquests.SessionClass-based settings (or similar dependency-injected configuration) are preferred over module-level globals. This keeps related state colocated with the class that owns it, makes the dependency surface explicit, and avoids hidden coupling between modules.
In tests, the same principle applies: prefer pytest fixtures over module-level globals for shared test setup that is non-trivial to construct. Fixtures make dependencies explicit at the test signature level, support scoping (function/module/session), and can be overridden or parametrized — all of which globals cannot. (For trivial values like a single string, inline them at the call site instead — see Section 4.)
Good — shared test setup is exposed via a fixture:
@pytest.fixture(name="parsed_document")
def _parsed_document() -> ParsedDocument:
return ParsedDocument(document_id="document-id", page_count=1, text="text")
def test_something(parsed_document: ParsedDocument):
...Bad — shared test setup defined as a module-level global:
_PARSED_DOCUMENT = ParsedDocument(document_id="document-id", page_count=1, text="text")
def test_something():
# implicitly depends on _PARSED_DOCUMENT
...When extending or merging built-in collections, prefer the operator form over the equivalent method call: += over list.extend, |= over dict.update and set.update, and | over {**a, **b} or copy-then-update when building a new dict/set.
Operators are more concise, read as a single expression, and make the intent (combine these collections) immediately visible without scanning for a method name. The semantics are equivalent for these cases.
Good
items += extra_items
settings |= overrides
merged = defaults | overridesBad
items.extend(extra_items)
settings.update(overrides)
merged = {**defaults, **overrides}Note that the augmented forms (+=, |=) accept any iterable/mapping on the right-hand side, exactly like extend/update, so they are drop-in replacements. Only the binary forms (a + b, a | b) require both operands to be the same built-in type — convert the operand first if needed.
Avoid defining def, class, or lambda inside another function. They are harder to test (you cannot import or call them directly), they hide their captured state in closure cells instead of explicit attributes (so tracebacks show opaque <locals> frames and the dependencies of the inner callable are invisible at a glance), and they push function bodies toward being long and hard to scan.
Prefer a module-level function, or — when the thing needs to capture state and be invoked later — a small class that holds that state as explicit attributes and exposes a __call__ (or named methods). The state becomes inspectable, the unit is testable in isolation, and the "factory + specialized callable" pattern reads as ordinary OO.
This is especially the shape to reach for when a factory builds a configured callable once and invokes it many times: resolve the configuration in __init__, do the work in __call__.
Bad — a factory returning a closure; the captured prefix/suffix live in invisible cells:
def make_wrapper(prefix: str, suffix: str) -> Callable[[str], str]:
def wrap(value: str) -> str:
return f"{prefix}{value}{suffix}"
return wrapGood — the captured state is explicit and the unit is testable on its own:
@dataclass(slots=True)
class Wrapper:
prefix: str
suffix: str
def __call__(self, value: str) -> str:
return f"{self.prefix}{value}{self.suffix}"The "almost" covers the genuinely trivial, single-use, local callable where promotion adds only noise — most commonly a key=/predicate passed inline to a builtin:
names.sort(key=lambda person: person.age)If the inner callable closes over state, is reused, is more than a line or two, or would benefit from a test, promote it. Note this rule is about def/class/lambda nesting — comprehensions and generator expressions are not "nested functions" and are unaffected.
Service-like classes are named singular (WidgetService, not WidgetsService), and every other name is derived mechanically from the class name — there is nothing to decide, so there is nothing to debate in review:
- Attribute / variable: the snake_case of the class —
WidgetService→_widget_service. (_servicealone is fine while a class holds exactly one.) - Module: a module that exists to hold one primary class mirrors it —
widget_service.pyholdsWidgetService. A module that is genuinely an area — several peer classes, no primary — is named for the area (errors.py,models.py,system_operations.py); that is a "no single class to mirror" fact, not a naming choice. - Test mock fixtures: the attribute name plus
_mock—widget_service_mock.
Singular-by-derivation never breaks on mass nouns (WeatherService → _weather_service); a "plural" convention immediately produces a mixed style (_widgets_service next to _weather_service), and a mixed style is worse than either pure one. Plural belongs where a collection actually lives: REST paths (/widgets) and methods returning many (list_widgets).
Good
# widget_service.py
class WidgetService: ...
# elsewhere
_widget_service: WidgetServiceBad
# widgets_service.py — module doesn't mirror the class
class WidgetsService: ... # plural class
_widgets: WidgetsService # attribute derived from nothingThe root of the class name follows one rule — name the class for what it abstracts:
- A
Resourceabstracts an entity type, so it is the singular entity:WidgetResource(even though it mounts at the plural path/widgets). This keeps the whole domain vertical under one greppable prefix:Widget,WidgetIn,WidgetPath,WidgetService,WidgetResource,WidgetNotFoundError,widget_service.py,widget_operations.py. - An
Endpointabstracts a path, so it is named for the path, spelled as the path spells it:NotificationsEndpointat/notifications,HealthEndpointat/healthz,WhoAmIEndpointat/me. Forcing singular here would produceNotificationEndpointserving/notifications— wrong twice. - A service abstracts behavior over a domain, so it is the singular domain noun:
WidgetService,WeatherService.