| 1 | # Python Code Style Guide |
| 2 | |
| 3 | > Style rules for Python code under `skills/ppt-master/scripts/` and any Python that ships with the skill. Derived from the de facto patterns in the existing codebase. |
| 4 | |
| 5 | These rules are pragmatic, not exhaustive. They capture the conventions readers actually encounter — anything PEP 8 hands you for free is assumed. |
| 6 | |
| 7 | --- |
| 8 | |
| 9 | ## 1. File Header |
| 10 | |
| 11 | Every script under `scripts/` starts with: |
| 12 | |
| 13 | ```python |
| 14 | #!/usr/bin/env python3 |
| 15 | """ |
| 16 | PPT Master - Short Tool Name |
| 17 | |
| 18 | One-paragraph description of what this script does. |
| 19 | |
| 20 | Usage: |
| 21 | python3 scripts/<name>.py <required_arg> [options] |
| 22 | |
| 23 | Examples: |
| 24 | python3 scripts/<name>.py projects/<project_name> -o output_dir |
| 25 | |
| 26 | Dependencies: |
| 27 | None (only uses standard library) <-- or list third-party deps |
| 28 | """ |
| 29 | ``` |
| 30 | |
| 31 | | Element | Rule | |
| 32 | |---|---| |
| 33 | | Shebang | `#!/usr/bin/env python3` (always — even for non-CLI helper modules) | |
| 34 | | Module docstring | Tool name + purpose + Usage + Examples + Dependencies | |
| 35 | | Internal helper modules | May add an early `--help` short-circuit (see §4) | |
| 36 | |
| 37 | --- |
| 38 | |
| 39 | ## 2. Imports |
| 40 | |
| 41 | ```python |
| 42 | # 1. Standard library |
| 43 | import os |
| 44 | import sys |
| 45 | import argparse |
| 46 | import re |
| 47 | from pathlib import Path |
| 48 | from typing import Optional |
| 49 | |
| 50 | # 2. Third-party |
| 51 | import requests |
| 52 | |
| 53 | # 3. Local — sometimes need sys.path injection first (see §3) |
| 54 | from image_sources.provider_common import ( |
| 55 | AssetCandidate, |
| 56 | ImageSearchRequest, |
| 57 | ) |
| 58 | ``` |
| 59 | |
| 60 | | Rule | Note | |
| 61 | |---|---| |
| 62 | | Group order | std → third-party → local, blank line between groups | |
| 63 | | Within a group | Sorted by length when short; alphabetical when ≥ 4 imports | |
| 64 | | `from x import` lists | One name per line if ≥ 4 names, with trailing comma | |
| 65 | | `from __future__ import annotations` | Add at top when the file uses `X \| Y` union syntax (PEP 604) and may run on Python < 3.10 | |
| 66 | |
| 67 | --- |
| 68 | |
| 69 | ## 3. sys.path Injection (Project Convention) |
| 70 | |
| 71 | `scripts/` is **not a Python package** — it's a flat directory of scripts. Each entry-point script that imports a sibling module injects `scripts/` onto `sys.path` itself: |
| 72 | |
| 73 | ```python |
| 74 | import sys |
| 75 | from pathlib import Path |
| 76 | |
| 77 | _SCRIPTS_DIR = Path(__file__).resolve().parent |
| 78 | if str(_SCRIPTS_DIR) not in sys.path: |
| 79 | sys.path.insert(0, str(_SCRIPTS_DIR)) |
| 80 | |
| 81 | from image_backends.backend_common import download_image # noqa: E402 |
| 82 | ``` |
| 83 | |
| 84 | | Rule | Why | |
| 85 | |---|---| |
| 86 | | Inject only in entry-points | Library modules under `image_sources/` / `image_backends/` import each other normally | |
| 87 | | Use `Path(__file__).resolve().parent` | Robust under symlinks and aliasing | |
| 88 | | Annotate post-injection imports with `# noqa: E402` | Suppress the lint warning honestly, not via per-file noqa | |
| 89 | |
| 90 | --- |
| 91 | |
| 92 | ## 4. CLI Entry Points |
| 93 | |
| 94 | ```python |
| 95 | def build_parser() -> argparse.ArgumentParser: |
| 96 | parser = argparse.ArgumentParser( |
| 97 | description="One-line description.", |
| 98 | formatter_class=argparse.RawDescriptionHelpFormatter, |
| 99 | ) |
| 100 | parser.add_argument("query", help="...") |
| 101 | parser.add_argument("-o", "--output", default=".", help="...") |
| 102 | return parser |
| 103 | |
| 104 | |
| 105 | def main(argv: Optional[list[str]] = None) -> int: |
| 106 | parser = build_parser() |
| 107 | args = parser.parse_args(argv) |
| 108 | # ... do the thing ... |
| 109 | return 0 |
| 110 | |
| 111 | |
| 112 | if __name__ == "__main__": |
| 113 | raise SystemExit(main()) |
| 114 | ``` |
| 115 | |
| 116 | | Rule | Note | |
| 117 | |---|---| |
| 118 | | `main(argv=None) -> int` | Returns exit code; testable by passing `argv` | |
| 119 | | `raise SystemExit(main())` | Preferred over `sys.exit(main())` | |
| 120 | | `formatter_class=argparse.RawDescriptionHelpFormatter` | Preserves docstring formatting in `--help` | |
| 121 | | Internal helpers `--help` | Module-level: `if __name__ == "__main__" and any(arg in {"-h", "--help", "help"} for arg in sys.argv[1:]): print(__doc__); raise SystemExit(0)` | |
| 122 | | Output | Progress / status to **stderr**; the script's primary output (if any) to stdout | |
| 123 | |
| 124 | **Help and argument validation requirements**: |
| 125 | |
| 126 | - `-h` / `--help` must be handled by `argparse` (preferred) or an explicit early helper guard before any side effect: no directory creation, file writes, network calls, package installs, or long-running servers. |
| 127 | - Help flags must never be accepted as positional values. Examples: `init --help` must not create a project named `--help`; `export --help` must not write a file named `--help`. |
| 128 | - Scripts with subcommands use `argparse` subparsers. Each subcommand must provide its own help (`<script> <subcmd> --help`) and validate its own required arguments before doing work. |
| 129 | - Missing required arguments and unknown flags must print a usage/error message and exit non-zero. Do not silently ignore unknown flags or continue with partial defaults. |
| 130 | - For internal helper modules that are directly executable only for diagnostics, non-help invocation should either run a documented diagnostic command or print a short "use via ..." message and exit non-zero; it must not fail with an import traceback. |
| 131 | |
| 132 | **Console encoding**: |
| 133 | |
| 134 | - Every directly-runnable entry script must call `configure_utf8_stdio()` (from `console_encoding.py`, §9) once at startup, before any user-facing print or before importing optional dependencies that may print. It forces `stdout`/`stderr` to UTF-8 with `errors="replace"` so non-UTF-8 Windows locales (e.g. GBK) do not crash on Unicode status output. On systems already using UTF-8 it leaves the effective encoding unchanged. |
| 135 | - Subdirectory scripts inject the scripts root onto `sys.path` first (§3), then import the helper. Pure library modules with no `__main__` entry do not need it. File I/O still passes `encoding="utf-8"` explicitly (§13); this rule only covers the console. |
| 136 | |
| 137 | --- |
| 138 | |
| 139 | ## 5. Type Hints |
| 140 | |
| 141 | Required for all new public functions; optional for internal `_helpers`. |
| 142 | |
| 143 | | Pattern | Use | |
| 144 | |---|---| |
| 145 | | `def f(x: str, *, y: int = 0) -> bool:` | Public functions | |
| 146 | | `tuple[int, int] \| None` | PEP 604 unions (with `from __future__ import annotations` if needed for compat) | |
| 147 | | `Optional[X]` from `typing` | Acceptable alternative to `X \| None` | |
| 148 | | `list[X]`, `dict[K, V]` | Built-in generics (Python 3.9+) | |
| 149 | | `Any` | Sparingly — only when interfacing with truly heterogeneous data (`raw: Any` in dataclasses for upstream JSON) | |
| 150 | |
| 151 | **Forbidden — over-specification**: |
| 152 | |
| 153 | - `Callable[[int, str], dict[str, list[Optional[Union[int, str]]]]]` — break this into typed dataclasses |
| 154 | - `Literal["a", "b", "c"]` everywhere — use a constant + plain `str` unless the type itself is the API |
| 155 | |
| 156 | --- |
| 157 | |
| 158 | ## 6. Naming |
| 159 | |
| 160 | | Kind | Convention | Examples | |
| 161 | |---|---|---| |
| 162 | | Module file | `snake_case.py` | `image_search.py`, `svg_to_pptx.py` | |
| 163 | | Script entrypoint | verb or noun phrase | `finalize_svg.py`, `notes_to_audio.py` | |
| 164 | | Public function | `snake_case` | `download_image`, `parse_results` | |
| 165 | | Private helper | `_snake_case` | `_load_dotenv_if_available`, `_measure_actual_image` | |
| 166 | | Constant | `UPPER_SNAKE_CASE` | `API_URL`, `DEFAULT_PAGE_SIZE`, `LICENSE_TIER_NO_ATTRIBUTION` | |
| 167 | | Class | `PascalCase` | `AssetCandidate`, `SVGQualityChecker` | |
| 168 | | Dataclass field | `snake_case` | `license_tier`, `download_url` | |
| 169 | | Module-private regex | `_PATTERN_RE` (private + `_RE` suffix) | `_TAG_RE`, `HEADING_RE` | |
| 170 | |
| 171 | --- |
| 172 | |
| 173 | ## 7. Error Handling |
| 174 | |
| 175 | | Situation | Pattern | |
| 176 | |---|---| |
| 177 | | Optional dependency | `try: import x; HAS_X = True\nexcept ImportError: HAS_X = False` | |
| 178 | | Optional sibling module | `try: from project_utils import CANVAS_FORMATS\nexcept ImportError: CANVAS_FORMATS = {}; print("Warning: ...")` | |
| 179 | | Recoverable runtime failure | Catch specific exceptions, log to stderr, return / continue — do NOT halt the pipeline | |
| 180 | | User-facing error | `print("...", file=sys.stderr); return 1` from `main()` | |
| 181 | | Programming error | Raise — don't paper over a bug | |
| 182 | |
| 183 | **Hard rule**: never bare-`except:`. Always name the exception class. |
| 184 | |
| 185 | **Forbidden — silent fallbacks for security-relevant code**: |
| 186 | |
| 187 | - Disabling SSL verification without a domain whitelist + WARNING |
| 188 | - Catching all exceptions in a download path without logging the cause |
| 189 | |
| 190 | --- |
| 191 | |
| 192 | ## 8. Dependencies |
| 193 | |
| 194 | | Tier | Where it can be required | |
| 195 | |---|---| |
| 196 | | Standard library | Anywhere | |
| 197 | | `requests`, `Pillow`, `lxml` | Common dependencies; safe to require in main scripts | |
| 198 | | Provider SDKs (`google-genai`, `openai`, `anthropic`, etc.) | **Lazy import inside the function that uses it**; soft-fail with `ImportError` → `RuntimeError` containing install instructions | |
| 199 | | `python-dotenv` | Optional — wrap the import in try/except, no-op if unavailable | |
| 200 | |
| 201 | ```python |
| 202 | def _require_api_key() -> str: |
| 203 | key = os.environ.get("PEXELS_API_KEY") or "" |
| 204 | if not key: |
| 205 | raise RuntimeError( |
| 206 | "PEXELS_API_KEY is not set. Add it to your environment or .env file. " |
| 207 | "Get one at https://www.pexels.com/api/" |
| 208 | ) |
| 209 | return key |
| 210 | ``` |
| 211 | |
| 212 | Error messages **must include the fix** — "what env var to set", "where to get a key", "which package to install". |
| 213 | |
| 214 | --- |
| 215 | |
| 216 | ## 9. Shared Helpers Layer |
| 217 | |
| 218 | Common functionality lives in these designated submodules. New scripts use these, not their own copies: |
| 219 | |
| 220 | | Module | Owns | |
| 221 | |---|---| |
| 222 | | [`image_backends/backend_common.py`](../../skills/ppt-master/scripts/image_backends/backend_common.py) | HTTP download, retry, image format detection, save-with-Pillow-transcode | |
| 223 | | [`image_sources/provider_common.py`](../../skills/ppt-master/scripts/image_sources/provider_common.py) | License classification, query simplification, scoring, attribution text, dataclasses | |
| 224 | | [`project_utils.py`](../../skills/ppt-master/scripts/project_utils.py) | Canvas formats, project path conventions | |
| 225 | | [`slide_roster.py`](../../skills/ppt-master/scripts/slide_roster.py) | Numeric slide-filename ordering and SVG roster discovery | |
| 226 | | [`error_helper.py`](../../skills/ppt-master/scripts/error_helper.py) | User-facing error message templates | |
| 227 | | [`console_encoding.py`](../../skills/ppt-master/scripts/console_encoding.py) | `configure_utf8_stdio()` — force UTF-8 console for CLI entries (§4) | |
| 228 | |
| 229 | **Forbidden — duplicating logic that exists in a shared helper**. If a helper is missing a feature, extend the helper, don't fork it inside your new script. |
| 230 | |
| 231 | --- |
| 232 | |
| 233 | ## 10. Docstrings |
| 234 | |
| 235 | Short and imperative. No Args/Returns/Raises sections unless the signature is genuinely complex. |
| 236 | |
| 237 | ```python |
| 238 | def classify_license( |
| 239 | license_name: str, |
| 240 | license_url: str = "", |
| 241 | provider: str = "", |
| 242 | ) -> Optional[str]: |
| 243 | """Classify a license string into one of the two tiers, or reject it. |
| 244 | |
| 245 | Returns: |
| 246 | ``"no-attribution"`` / ``"attribution-required"`` / ``None``. |
| 247 | |
| 248 | The provider hint lets us treat Pexels and Pixabay's own licenses as |
| 249 | ``no-attribution`` even when the upstream API only returns a short |
| 250 | label like ``"Pexels"``. |
| 251 | """ |
| 252 | ``` |
| 253 | |
| 254 | | Use a Google/Sphinx-style block | Skip the block | |
| 255 | |---|---| |
| 256 | | Function returns multiple branches with semantic differences | One-liner that explains itself in the function name | |
| 257 | | Has more than 3 parameters with non-obvious roles | Single-purpose helper | |
| 258 | | Maintains a non-trivial invariant | Pure formatter / accessor | |
| 259 | |
| 260 | --- |
| 261 | |
| 262 | ## 11. Testing |
| 263 | |
| 264 | **Hard rule**: this repository does **not** ship automated tests. |
| 265 | |
| 266 | **Forbidden**: |
| 267 | |
| 268 | - `tests/` directories |
| 269 | - `test_*.py` files |
| 270 | - `unittest` / `pytest` imports |
| 271 | - `if __name__ == "__main__":` blocks that run a self-test suite |
| 272 | |
| 273 | **Use instead**: |
| 274 | |
| 275 | - Inline smoke commands via `python3 -c "..."` against real project samples; show the output in the conversation / PR description |
| 276 | - Manual verification steps in the runbook |
| 277 | - Live-API smoke runs against `projects/_smoke_*` directories (gitignored) |
| 278 | |
| 279 | This is a deliberate project convention. When external contributors include tests, ask them to remove tests in PR review (see [`docs/rules/prompt-style.md`](./prompt-style.md) §11 for the parallel rule on reference docs). |
| 280 | |
| 281 | --- |
| 282 | |
| 283 | ## 12. Dataclasses |
| 284 | |
| 285 | Prefer plain `@dataclass` over `pydantic` / `attrs` for value types. Keep them simple: |
| 286 | |
| 287 | ```python |
| 288 | @dataclass |
| 289 | class AssetCandidate: |
| 290 | provider: str |
| 291 | title: str |
| 292 | asset_id: str = "" |
| 293 | license_tier: str = "" |
| 294 | width: int = 0 |
| 295 | height: int = 0 |
| 296 | raw: Any = None |
| 297 | ``` |
| 298 | |
| 299 | | Rule | Note | |
| 300 | |---|---| |
| 301 | | `@dataclass` | Default; no need for `frozen=True` unless mutation is a real risk | |
| 302 | | Fields | All required fields first, then optional with defaults | |
| 303 | | `field(default_factory=...)` | Only when the default needs to be a new container per instance | |
| 304 | | No legacy positional-arg shims | New dataclass = keyword-arg API. YAGNI on positional support | |
| 305 | | Methods | Keep dataclasses dumb; computation goes in module-level functions | |
| 306 | |
| 307 | --- |
| 308 | |
| 309 | ## 13. File Encoding & Line Endings |
| 310 | |
| 311 | | Property | Value | |
| 312 | |---|---| |
| 313 | | Encoding | UTF-8 | |
| 314 | | Line endings | LF | |
| 315 | | Final newline | Always present | |
| 316 | | BOM | Forbidden | |
| 317 | | Indentation | 4 spaces (no tabs) | |
| 318 | | Max line length | Soft 100; hard 120. Prose in docstrings can flow longer | |
| 319 | |
| 320 | --- |
| 321 | |
| 322 | ## 14. Cross-references |
| 323 | |
| 324 | When a Python file mirrors a reference doc, cross-link both ways: |
| 325 | |
| 326 | - The script's docstring mentions the reference: `See references/image-searcher.md for the on-slide attribution rules.` |
| 327 | - The reference doc cites the script with a backticked relative link |
| 328 | |
| 329 | This keeps `prompt-style.md` and `code-style.md` (this file) operating as a pair — neither layer drifts away from the other. |
| 330 | |
| 331 | --- |
| 332 | |
| 333 | ## 15. When This Guide Conflicts With Existing Files |
| 334 | |
| 335 | Existing files take precedence. If a current script contradicts a rule here, decide whether to (a) update this guide, or (b) refactor the script. The canonical exemplars to model new scripts after: |
| 336 | |
| 337 | | If you're writing... | Model after | |
| 338 | |---|---| |
| 339 | | A small CLI utility | [`total_md_split.py`](../../skills/ppt-master/scripts/total_md_split.py), [`gemini_watermark_remover.py`](../../skills/ppt-master/scripts/gemini_watermark_remover.py) | |
| 340 | | A multi-backend / dispatcher CLI | [`image_search.py`](../../skills/ppt-master/scripts/image_search.py), [`image_gen.py`](../../skills/ppt-master/scripts/image_gen.py) | |
| 341 | | A library / shared helper | [`image_sources/provider_common.py`](../../skills/ppt-master/scripts/image_sources/provider_common.py), [`image_backends/backend_common.py`](../../skills/ppt-master/scripts/image_backends/backend_common.py) | |
| 342 | | A class-based checker / validator | [`svg_quality/checker.py`](../../skills/ppt-master/scripts/svg_quality/checker.py) | |
| 343 |