amdsmi-python-style-guide

name	amdsmi-python-style-guide
description	ROCm Python style guide based on TheRock. Use when: writing Python code, reviewing Python PRs, checking Python style, creating Python scripts, type hints, error handling patterns, pathlib usage, argparse CLI design.

Python Style Guide (ROCm / TheRock)

Follows PEP 8, enforced by Ruff (amd-smi) or Black (TheRock) via pre-commit. Source: TheRock python_style_guide.md

Core Principles

• Fail fast and loud — false positives over silent corruption
• When in doubt, raise an exception

---

Type Hints

• Type hints on all function signatures
• Syntax that is compatible with python 3.6.8+
• Never use Any except in rare generic code
• Never use Optional[T], Union[X, Y], List, Dict — use built-in equivalents
• Do NOT use from __future__ import annotations

Extract Complex Types

• Type appears in multiple signatures → NamedTuple or TypeAlias
• Dict/tuple with 3+ fields → dataclass or NamedTuple
• If you document what tuple fields mean → use NamedTuple

# Good
class KpackInfo(NamedTuple):
    kpack_path: Path
    size: int
    kernel_count: int

# Bad
def get_info() -> tuple[Path, int, int]: ...

Dataclasses Over Tuples

Structured data with multiple fields → @dataclass or NamedTuple, not raw tuples. Tuples OK for: simple pairs (x, y), stdlib unpacking, single-use immediate unpacking.

---

Function Calls & Arguments

• 3+ parameters or ambiguous types → use keyword arguments
• Multiple booleans → always use keyword arguments

# Good
result = build(amdgpu_family="gfx942", enable_testing=True, use_ccache=False)

# Bad
result = build("gfx942", True, False)

---

Code Organization

• All imports at top of file — inline only for documented circular deps (add comment explaining why)
• Scripts must have if __name__ == "__main__": guard
• CLI scripts must use argparse (not raw sys.argv)
• Access argparse attrs directly: args.foo, not getattr(args, "foo", default)
• No duplicate code — extract to shared functions
• No magic numbers or fake estimates
• Methods < 30 lines, classes < 200 lines (ideally)
• If a class has 7+ responsibilities → split into focused classes
• God objects doing everything → multiple focused classes; 100+ line methods → extract helpers

---

Error Handling

Rule	Detail
Fail fast	Never silently continue or produce incomplete results
Specific exceptions	FileNotFoundError, subprocess.CalledProcessError — not bare except Exception
Preserve chains	Always use from e when re-raising
Validate results	After critical ops: check file exists, non-empty, expected size
No binutils timeouts	Never timeout= on readelf, objcopy, etc.

# Good — distinguish error conditions
try:
    output = subprocess.check_output([readelf, "-S", str(file_path)])
except subprocess.CalledProcessError as e:
    if e.returncode == 1:
        return False  # legitimate "not found"
    raise RuntimeError(f"readelf failed: {e.output}") from e
except FileNotFoundError as e:
    raise RuntimeError(f"readelf not found: {readelf}") from e

# Bad — swallows everything
try:
    output = subprocess.check_output([readelf, "-S", str(file_path)])
except Exception:
    return False

---

Filesystem & Paths

• pathlib.Path everywhere — no os.path, no string concatenation for paths
• No assumptions about CWD — derive from Path(__file__).resolve().parent
• No hard-coded project paths — use env vars, tempfile, or relative to __file__

# Good
THIS_DIR = Path(__file__).resolve().parent
config = THIS_DIR / "config.json"

# Bad
config = Path("build_tools/config.json")  # assumes CWD
config = Path("/home/user/project/config.json")  # hard-coded

---

Performance

• Compile regexes at module level, not inside functions
• Check cheap conditions before expensive ones (magic bytes before subprocess)
• Cache expensive computations when called repeatedly
• Use generators for large datasets

---

Testing

• Verify fail-fast behavior with pytest.raises
• Use real temp files over mocks for filesystem operations
• Mock only external dependencies (network, expensive tools)
• Integration tests should exercise the full path

---

amd-smi Python Conventions

Naming

• py-interface/amdsmi_interface.py mirrors C API names (amdsmi_get_*, amdsmi_set_*)
• amdsmi_cli/ uses snake_case methods
• Private functions/methods: _ prefix

Project Layout

Directory	Purpose
py-interface/	Python bindings — amdsmi_interface.py, amdsmi_wrapper.py, amdsmi_exception.py
amdsmi_cli/	CLI tool — amdsmi_commands.py, amdsmi_parser.py, amdsmi_helpers.py

Critical Python Files (high churn — review carefully)

• amdsmi_cli/amdsmi_commands.py — CLI behavior regressions, output format changes
• py-interface/amdsmi_interface.py — must stay in sync with C header amdsmi.h
• py-interface/amdsmi_wrapper.py — generated bindings + library loader (see loader rules below)
• amdsmi_cli/amdsmi_parser.py — argument parsing

Library Loader Rules (amdsmi_wrapper.py)

Changes to _detect_install_context, _build_candidate_paths, or _load_library are ❌ BLOCKING if they break system or pip install context. Verify:

• Path(__file__).resolve() used correctly
• pip detection logic intact
• _libraries['libamd_smi.so'] key preserved
• Sync with tools/generator.py

Python Testing

• Tests must work with both system-installed and pip-installed amdsmi
• CLI tests live in amdsmi_cli/
• Verify fail-fast behavior with pytest.raises
• Use real temp files over mocks for filesystem operations
• Mock only external dependencies (network, expensive tools)
• Integration tests should exercise the full path

---

Review Checklist

When reviewing Python code, verify:

• Type hints on all functions — modern syntax, no Any
• No from __future__ import annotations
• Complex types extracted to NamedTuple/dataclass
• Specific exception handling — no bare except Exception
• Fail-fast — no silent continue on errors
• from e on all re-raises
• pathlib.Path for all filesystem ops
• No CWD assumptions, no hard-coded paths
• No magic numbers or fake estimates
• No duplicate code
• argparse for CLI, __main__ guard for scripts
• Keyword args for complex function calls
• All imports at top (inline only with circular-dep comment)
• No timeouts on binutils
• Output validation after critical operations
• Methods < 30 lines, classes < 200 lines