improving-python-code-quality

Python のコード品質

クイックリファレンス

ツール	目的	コマンド
ruff	リント＋フォーマット	ruff check src && ruff format src
mypy	型チェック	mypy src

Ruff の設定

pyproject.toml の最小限の設定：

[tool.ruff]
line-length = 88
target-version = "py310"

[tool.ruff.lint]
select = ["E", "W", "F", "I", "B", "C4", "UP"]

設定オプションの詳細については、RUFF_CONFIG.md を参照してください。

MyPy の設定

[tool.mypy]
python_version = "3.10"
disallow_untyped_defs = true
warn_return_any = true

厳格な設定とオーバーライドについては、MYPY_CONFIG.md を参照してください。

型ヒントのパターン

# Basic
def process(items: list[str]) -> dict[str, int]: ...

# Optional
def fetch(url: str, timeout: int | None = None) -> bytes: ...

# Callable
def apply(func: Callable[[int], str], value: int) -> str: ...

# Generic
T = TypeVar("T")
def first(items: Sequence[T]) -> T | None: ...

プロトコルと高度なパターンについては、TYPE_PATTERNS.md を参照してください。

よくあるアンチパターン

# Bad: Mutable default
def process(items: list = []):  # Bug!
    ...

# Good: None default
def process(items: list | None = None):
    items = items or []
    ...

# Bad: Bare except
try:
    ...
except:
    pass

# Good: Specific exception
try:
    ...
except ValueError as e:
    logger.error(e)

# Bad: truthiness guard swallows a legitimate 0 / 0.0 / "" / False
hour = config.get("start_hour") or 9   # a valid 0 (midnight) silently becomes 9
if self.max_drawdown:                  # max_drawdown=0 silently disables the limit
    enforce(self.max_drawdown)

# Good: guard on None, not on truthiness
hour = config.get("start_hour")
hour = 9 if hour is None else hour
if self.max_drawdown is not None:
    enforce(self.max_drawdown)

x = x or default が適切なのは、偽とみなされる値が意味のある入力にならない場合に限ります。つまり、対象が None またはオブジェクトである場合、あるいは唯一の偽値が空のコンテナーである場合です（例：items = items or []）。0、False、"" が意味のある入力となる数値、ブール値、文字列のフィールドでは、これはバグです。x if x is not None else default を使ってください。

# Bad: identity comparison against a literal (ruff flags this as F632)
if name is not "":   # CPython interning makes it *sometimes* work — never rely on it

# Good: value comparison
if name != "":

有効にしたルールが実際に動作するとは限りません

リンターには例外設定があり、厄介なのは目に見えないものです。ルールが select に含まれ、ジョブも成功しているのに、検出されると思っていた種類のミスがそのまま通り抜けます。チェックに何かを守らせる前に、意図的に一度違反を記述し、報告されることを確認してください。

Ruff のデフォルト設定で特に問題になりやすいのは dummy-variable-rgx です。_ で始まる使い捨て変数が未使用変数のチェックに引っかからないようにするものですが、デフォルトのパターンは先頭にアンダースコアが付くすべての名前に一致します。そのため、F811（未使用名の再定義）はコードベース内のすべてのプライベートヘルパーを無視します。

# module.py — two module-level definitions. The FIRST one is dead code.
def _is_rate_limited(response):          # never called; editing it has no effect
    return response.status_code == 403

def _is_rate_limited(response):          # this is the one that wins
    return response.status_code == 403 and "rate limit" in response.text.lower()

そのファイルに対して ruff check --select F811 を実行しても、問題なく通ります。両方の名前を is_rate_limited に変更すると、すぐに検出されます（ruff 0.16 で確認済み）。チェック自体は機能しています。名前がチェック対象から外れていたのです。

このパターンが最もよく見られるのは、2 つのブランチでそれぞれ同じモジュールレベルのヘルパーが追加され、競合の解決時に両方が残った場合です。何も失敗せず、最初のうちは通常、両方の本体も一致しています。その後、誰かが使われない方のコピーを修正しても、なぜ動作が変わらないのか分からなくなります。

プライベート名もチェック対象になるよう、設定で修正することをおすすめします。

[tool.ruff.lint]
dummy-variable-rgx = "^_$"   # only a bare `_` is a throwaway

採用する前に、トレードオフを正直に説明してください。_, keep = pair() は引き続き警告されませんが、_unused = compute() は F841 に引っかかるようになります。_name 形式の使い捨て変数を多用するコードベースでは、これは無視できないノイズです。正規表現を元に戻すのではなく、代入を削除するか、単独の _ を使ってください。

これとは別に、両方のブランチで編集されたモジュールの競合を解決した後は、モジュールレベルの定義が重複していないかを直接確認してください。リンターの設定では検出できないシャドーイングを見つけられます。パターンが行頭に固定されているのは意図的です。クラスや関数の中にあるネストされた def／class はインデントされるため一致しません。偶発的なシャドーイングが実際に問題となるのは、この範囲です。

grep -oE '^(def|class) [A-Za-z_][A-Za-z0-9_]*' module.py | sort | uniq -d

この習慣は Ruff 以外にも応用できます。重要なチェックについては、意図的に一度ミスを起こし、検出されることを確認してください。回帰テストは失敗するところを見て初めて信頼できるのと同じです。

失敗は明確に示す：静かに劣化させない

最もコストのかかるバグはクラッシュではなく、成功したように見える失敗です。エラーを握りつぶしてもっともらしい値を返すコードは、問題が起きたことを示す情報がないまま、後続処理のデータを破損させます。静かなフォールバックよりも、例外を発生させるか、少なくとも明示的なエラーを表面化させてください。

例外を汎用的な文字列にまとめないでください。また、出力を破棄しないでください。

# Bad: real parse error becomes a one-line string; traceback and context lost
try:
    return parse(path)
except Exception as e:
    return {"error": str(e)}     # every distinct failure looks the same

# Bad: a nonzero exit discards stdout — where many CLIs write their real summary
result = subprocess.run(cmd, capture_output=True, text=True)
if result.returncode != 0:
    return f"Error: {result.stderr}"   # tools that exit nonzero *by design*
                                       # (e.g. "N findings") report empty here

# Good: let it raise (or return stdout+stderr+code so the caller can decide)
return {"stdout": result.stdout, "stderr": result.stderr, "code": result.returncode}

成功と失敗には、明確に異なるセンチネルを使ってください。 エラー処理と成功処理で同じステータス値を書き込むと、失敗した実行が完了したように見えてしまいます。

# Bad: on failure, status is set to the SAME value that means "done"
try:
    run_job()
    status = "READY"
except Exception:
    status = "READY"     # failures are now indistinguishable from success

# Good: a terminal error state the UI/caller can branch on
    status = "ERROR"

失敗時に捏造したデータで置き換えないでください。 取得に失敗した際にランダムなデータやサンプルデータを返すと、UI にもっともらしいものの、実際には存在しない数値が表示されます。

# Bad: a network hiccup silently becomes made-up numbers
try:
    return fetch_metrics()
except RequestError:
    return generate_sample_data()   # user can't tell real from fake

# Good: propagate the failure (or return an explicit sentinel the UI renders as an error)

部分的な結果はそのように示してください。完全な結果として返してはいけません。 ページネーションによる取得が途中で中断され、取得できた分だけを返すと、全件を取得した結果と見分けがつきません。

# Bad: caller can't distinguish "12 items" from "12 of 900 before the API died"
def fetch_all():
    items = []
    for page in paginate():
        try:
            items.extend(page)
        except RequestError:
            break            # silent truncation
    return items

# Good: return completeness alongside the data (or raise)
    return items, complete   # caller warns loudly when complete is False

バッチ処理のループでは、単に continue するのではなく、項目ごとのエラーを収集してください。 不正な入力を黙ってスキップすると、処理範囲が不完全だったことを知る手段がありません。また、エラーを報告する操作としない操作が混在し、関連する処理間の一貫性も失われます。

# Bad: files that fail to load vanish with no trace
for f in files:
    try:
        process(load(f))
    except Exception:
        continue                     # how many were skipped? which ones?

# Good: accumulate skips and return them so callers (and automation) can see them
errors = []
for f in files:
    try:
        process(load(f))
    except Exception as e:
        errors.append({"file": f, "error": str(e)})
return {"processed": ..., "errors": errors}

共通する原則は、エラーを捕捉したら、意味のある方法で回復するか、失敗を見える形にする（例外を発生させる、エラーレベルでログを記録する、または区別可能なセンチネルを返す）ことです。except 内の return／continue／フォールバックによって、正常に見える出力を生成する箇所に、静かな破損が潜んでいます。

決定性と再現性

非決定的な出力は、気づかれにくいコストをもたらします。頻繁に変わってレビューしにくい差分、不安定なテスト、そしてシードから誰も再現できないシミュレーションです。原因としてよく現れるものが3つあります。

順序のないコンテナーからシリアライズする。 set を反復してリストや JSON ペイロードを構築したり、dict にマージしてから順に取り出したりすると、実行ごとに異なる順序で結果が出力されます。str のハッシュはプロセスごとにランダム化されるため、ファイルを再生成すると同じデータが並べ替えられ、本来の変更がノイズに埋もれてしまいます。差分をきれいに保つことが目的のリポジトリなら、なおさらです。シリアライズする前に、全体を一意に並べ替えてください。

# Bad: set iteration order isn't stable across runs
tags = [render(t) for t in tag_set]
json.dump(record, f)                     # nested sets/merges churn the output

# Good: sort before writing
tags = [render(t) for t in sorted(tag_set)]
json.dump(record, f, sort_keys=True)

複数の RNG があり、どれも注入できない。 random と numpy.random の両方から値を取得するライブラリは、再現可能にするために両方へシードを設定する必要があります。片方だけにシードを設定しても、もう片方は独自に動作するため、実行の決定性は半分にとどまります。また、グローバル RNG（random.seed、np.random.seed）にシードを設定すると、呼び出し元のグローバル状態を壊してしまいます。呼び出し元がグローバル状態に触れずに再現性を得られるよう、シード（または RNG インスタンス）を受け取り、ローカルジェネレーターを処理に渡してください。

# Bad: two independent global RNGs, no way to seed from the outside
def simulate():
    x = random.random()              # stdlib global stream
    y = np.random.normal()           # numpy global — a *separate* stream

# Good: caller-supplied, local generators; one seed reproduces the whole run
def simulate(seed: int | None = None):
    rng = random.Random(seed)
    nprng = np.random.default_rng(seed)
    x = rng.random()
    y = nprng.normal()

解析コンテキストが固定されていない。 ロケールに依存する解析、たとえば %b／%a（月名／曜日名）を使う datetime.strptime は、マシンによって動作が暗黙に変わります。フォーマットとロケールを固定し、自分のマシンで通るテストが CI でも通るようにしてください。

非決定性はテストも弱めます。出力を shuffle する関数では、アサーションが「これらの単語のいずれかを含む」といった緩いものになり、回帰を検出できなくなります。RNG を注入して境界を決定的にし、出力を正確に検証してください。

Python らしいイディオム

# Iteration
for item in items:           # Not: for i in range(len(items))
for i, item in enumerate(items):  # When index needed

# Dictionary access
value = d.get(key, default)  # Not: if key in d: value = d[key]

# Context managers
with open(path) as f:        # Not: f = open(path); try: finally: f.close()

# Comprehensions (simple only)
squares = [x**2 for x in numbers]

モジュール構成

src/my_library/
├── __init__.py      # Public API exports
├── _internal.py     # Private (underscore prefix)
├── exceptions.py    # Custom exceptions
├── types.py         # Type definitions
└── py.typed         # Type hint marker

チェックリスト

Code Quality:
- [ ] ruff check passes
- [ ] mypy passes (strict mode)
- [ ] Public API has type hints
- [ ] Public API has docstrings
- [ ] No mutable default arguments
- [ ] Specific exception handling
- [ ] Failures fail loud — no fabricated fallbacks, colliding success/error sentinels, or silently-truncated results
- [ ] Batch loops collect per-item errors instead of a bare `continue`
- [ ] Truthiness guards don't swallow valid 0/False/"" (guard on `is None`)
- [ ] No `is`/`is not` against literals (use ==/!=)
- [ ] Load-bearing lint rules verified to fire (F811 skips `_`-prefixed names under the default `dummy-variable-rgx`)
- [ ] No duplicate module-level `def`/`class` names after a conflict resolution
- [ ] Deterministic output: sort before serializing; seed/inject all RNGs (both `random` and `numpy`)
- [ ] py.typed marker present

詳細情報

このスキルは、Will McGinnis による Guide to Developing High-Quality Python Libraries の「Code Quality」のセクションに基づいています。さらに詳しく知りたい方は、以下の記事をご覧ください。

• Ruff によるリンティングとフォーマット
• McCabe 複雑度を理解する
• 型ヒントの追加