docs(review): Python adaptation files for clean-code, solid and code-review
The canonical Python stack has been pinned in ~/dev/.context/AGENT.md since local-dev v0.3.1, and three skills still had Go examples with no Python counterpart. Following the structure #18 settled on, and the existing solid/references/go-adaptation.md precedent, each gets a reference file rather than inline dilution of the primary doc. The stack table itself is NOT copied into any of them — each cites AGENT.md as its home. This repo spent today learning what a second copy of a fact costs. Content is the part a linter cannot give you: - clean-code: `raise X from e` as the analogue of `%w` error wrapping, dataclass over `dict[str, Any]`, and the two traps where the broken version reads tidier — mutable default arguments, and `if not amount:` for a value where zero is legitimate. - solid: `Protocol` over `ABC`, defined at the consumer, which is what makes DIP real in Python; plus what does NOT carry over from the Go rewrite (return structs/accept interfaces, embedding, package-as-deployment-unit). - code-review: a Python anti-pattern table parallel to the Go one, opening with the rule that a finding ruff or mypy already catches is not a review comment, it is a question about why the gate is not running. Refs #20 Bump-Type: minor
This commit is contained in:
@@ -0,0 +1,104 @@
|
||||
# SOLID Principles: Python Adaptation
|
||||
|
||||
`references/go-adaptation.md` rewrites each principle for Go. This file does the same
|
||||
for Python, and marks the places where the Go rewrite's advice does *not* carry over.
|
||||
|
||||
The canonical Python stack lives in `~/dev/.context/AGENT.md`; it is not restated here.
|
||||
|
||||
## The one idea that changes everything: `Protocol`
|
||||
|
||||
Go's interfaces are structural — a type satisfies one by having the methods, with no
|
||||
declaration. Python has the same thing since 3.8, and it is the right default:
|
||||
|
||||
```python
|
||||
from typing import Protocol
|
||||
|
||||
class InvoiceStore(Protocol):
|
||||
def get(self, invoice_id: InvoiceId) -> Invoice: ...
|
||||
def save(self, invoice: Invoice) -> None: ...
|
||||
```
|
||||
|
||||
- **Define the Protocol where it is *consumed*, not where it is implemented.** This is
|
||||
Go's "accept interfaces, return structs" and it is what makes DIP real: the high-level
|
||||
module owns the abstraction, and the adapter depends on it rather than the reverse.
|
||||
- **Prefer `Protocol` to `ABC`.** An ABC forces implementers to inherit, which couples
|
||||
them to your package and makes a third-party class impossible to adapt without a
|
||||
wrapper. Use an ABC only when you genuinely need shared implementation, not just a
|
||||
contract.
|
||||
- Add `@runtime_checkable` only if you actually call `isinstance` — and if you are
|
||||
calling `isinstance` against a Protocol, check whether the real problem is a missing
|
||||
polymorphic method (an LSP smell).
|
||||
|
||||
## S — Single Responsibility
|
||||
|
||||
Same as Go: the unit is the module, not just the class. `utils.py` is the Python god
|
||||
package — it cannot have one reason to change because it has no subject.
|
||||
|
||||
Python-specific tell: a class whose `__init__` stores arguments and whose only other
|
||||
method is `run()` or `execute()`. That is a function with ceremony. Write the function.
|
||||
|
||||
## O — Open/Closed
|
||||
|
||||
Extension in Python usually means **passing behaviour in**, not subclassing:
|
||||
|
||||
```python
|
||||
# Closed for modification: adding a rule does not touch this function
|
||||
def apply_rules(invoice: Invoice, rules: Sequence[Rule]) -> Invoice: ...
|
||||
```
|
||||
|
||||
`functools.singledispatch` is the idiomatic open extension point when dispatch is on
|
||||
type and the set of types grows outside your module. Beware the alternative that looks
|
||||
Pythonic and is not: monkey-patching is modification pretending to be extension, and it
|
||||
is invisible at the call site.
|
||||
|
||||
## L — Liskov
|
||||
|
||||
The Python-specific violation is **signature widening/narrowing that the type checker
|
||||
would catch if you let it**. Under `mypy --strict`, an override that narrows a parameter
|
||||
type or widens a return type is an error — which means most LSP violations in Python are
|
||||
type errors you chose not to see. This is the strongest argument for `--strict` in a
|
||||
design document rather than a style one.
|
||||
|
||||
Raising a *new* exception type in an override is the violation `mypy` will not catch:
|
||||
callers wrote `except StoreError`, and your subclass raises `TimeoutError`. Exceptions
|
||||
are part of the contract; Python just will not check them for you.
|
||||
|
||||
## I — Interface Segregation
|
||||
|
||||
Protocols make this cheap: define the narrow one you need at the call site.
|
||||
|
||||
```python
|
||||
class InvoiceReader(Protocol):
|
||||
def get(self, invoice_id: InvoiceId) -> Invoice: ...
|
||||
```
|
||||
|
||||
A function that only reads should accept `InvoiceReader`, not the full `InvoiceStore`.
|
||||
The test double then implements one method instead of eight, which is the practical
|
||||
payoff and the reason this principle survives contact with real code.
|
||||
|
||||
## D — Dependency Inversion
|
||||
|
||||
Constructor injection, exactly as in Go. Two Python-specific hazards:
|
||||
|
||||
- **Import-time side effects are hidden dependencies.** A module that opens a database
|
||||
connection or reads an env var at import has a dependency nothing can inject and no
|
||||
test can replace. Configuration belongs in `pydantic-settings` objects passed in.
|
||||
- **Default arguments are compile-time bindings.** `def send(client: Client = Client())`
|
||||
constructs the default once, at definition, and welds it to every caller. Default to
|
||||
`None` and construct inside, or require it.
|
||||
|
||||
## What does not carry over from the Go rewrite
|
||||
|
||||
- **"Return structs, accept interfaces"** — the returning half is about Go's zero values
|
||||
and nil interfaces. In Python, returning a Protocol type is fine and often better.
|
||||
- **Small interfaces because of embedding** — Python has no embedding; composition is
|
||||
explicit delegation, which is more verbose but reads honestly.
|
||||
- **Package-level responsibility as the unit of deployment** — Python packaging is much
|
||||
looser. The module boundary is still the right unit for *reasoning*, but do not expect
|
||||
it to be enforced.
|
||||
|
||||
## See also
|
||||
|
||||
- `references/solid-principles.md` — the canonical, language-neutral reference
|
||||
- `references/go-adaptation.md` — the Go rewrite; contrast is instructive
|
||||
- `clean-code/references/python-adaptation.md` — style-level Python conventions
|
||||
Reference in New Issue
Block a user