|
| 1 | +--- |
| 2 | +name: dignified-python-313 |
| 3 | +description: This skill should be used when editing Python code in the erk codebase. Use when writing, reviewing, or refactoring Python to ensure adherence to LBYL exception handling patterns, Python 3.13+ type syntax (list[str], str | None), pathlib operations, ABC-based interfaces, absolute imports, and explicit error boundaries at CLI level. Also provides production-tested code smell patterns from Dagster Labs for API design, parameter complexity, and code organization. Essential for maintaining erk's dignified Python standards. |
| 4 | +--- |
| 5 | + |
| 6 | +# Dignified Python - Python 3.13+ Coding Standards |
| 7 | + |
| 8 | +Write explicit, predictable code that fails fast at proper boundaries. |
| 9 | + |
| 10 | +--- |
| 11 | + |
| 12 | +## Quick Reference - Check Before Coding |
| 13 | + |
| 14 | +| If you're about to write... | Check this rule | |
| 15 | +| --------------------------------------------- | -------------------------------------------------------------------------------------------------------- | |
| 16 | +| `try:` or `except:` | → [Exception Handling](#1-exception-handling---never-for-control-flow-) - Default: let exceptions bubble | |
| 17 | +| `from __future__ import annotations` | → **FORBIDDEN** - Python 3.13+ doesn't need it | |
| 18 | +| `List[...]`, `Dict[...]`, `Union[...]` | → Use `list[...]`, `dict[...]`, `X \| Y` | |
| 19 | +| `dict[key]` without checking | → Use `if key in dict:` or `.get()` | |
| 20 | +| `path.resolve()` or `path.is_relative_to()` | → Check `path.exists()` first | |
| 21 | +| `typing.Protocol` | → Use `abc.ABC` instead | |
| 22 | +| `from .module import` | → Use absolute imports only | |
| 23 | +| `__all__ = ["..."]` in `__init__.py` | → See references/core-standards.md#code-in-**init**py-and-**all**-exports | |
| 24 | +| `print(...)` in CLI code | → Use `click.echo()` | |
| 25 | +| `subprocess.run(...)` | → Add `check=True` | |
| 26 | +| `@property` with I/O or expensive computation | → See references/core-standards.md#performance-expectations | |
| 27 | +| Function with many optional parameters | → See references/code-smells-dagster.md | |
| 28 | +| `repr()` for sorting or hashing | → See references/code-smells-dagster.md | |
| 29 | +| Context object passed everywhere | → See references/code-smells-dagster.md | |
| 30 | +| Function with 10+ local variables | → See references/code-smells-dagster.md | |
| 31 | +| Class with 50+ methods | → See references/code-smells-dagster.md | |
| 32 | + |
| 33 | +--- |
| 34 | + |
| 35 | +## CRITICAL RULES (Top 6) |
| 36 | + |
| 37 | +### 1. Exception Handling - NEVER for Control Flow 🔴 |
| 38 | + |
| 39 | +**ALWAYS use LBYL (Look Before You Leap), NEVER EAFP** |
| 40 | + |
| 41 | +```python |
| 42 | +# ✅ CORRECT: Check before acting |
| 43 | +if key in mapping: |
| 44 | + value = mapping[key] |
| 45 | +else: |
| 46 | + handle_missing_key() |
| 47 | + |
| 48 | +# ❌ WRONG: Using exceptions for control flow |
| 49 | +try: |
| 50 | + value = mapping[key] |
| 51 | +except KeyError: |
| 52 | + handle_missing_key() |
| 53 | +``` |
| 54 | + |
| 55 | +**Details**: See `references/core-standards.md#exception-handling` for complete patterns |
| 56 | + |
| 57 | +### 2. Type Annotations - Python 3.13+ Syntax Only 🔴 |
| 58 | + |
| 59 | +**FORBIDDEN**: `from __future__ import annotations` |
| 60 | + |
| 61 | +```python |
| 62 | +# ✅ CORRECT: Modern Python 3.13+ syntax |
| 63 | +def process(items: list[str]) -> dict[str, int]: ... |
| 64 | +def find_user(id: int) -> User | None: ... |
| 65 | + |
| 66 | +# ❌ WRONG: Legacy syntax |
| 67 | +from typing import List, Dict, Optional |
| 68 | +def process(items: List[str]) -> Dict[str, int]: ... |
| 69 | +``` |
| 70 | + |
| 71 | +**Details**: See `references/core-standards.md#type-annotations` for all patterns |
| 72 | + |
| 73 | +### 3. Path Operations - Check Exists First 🔴 |
| 74 | + |
| 75 | +```python |
| 76 | +# ✅ CORRECT: Check exists first |
| 77 | +if path.exists(): |
| 78 | + resolved = path.resolve() |
| 79 | + |
| 80 | +# ❌ WRONG: Using exceptions |
| 81 | +try: |
| 82 | + resolved = path.resolve() |
| 83 | +except OSError: |
| 84 | + pass |
| 85 | +``` |
| 86 | + |
| 87 | +**Details**: See `references/core-standards.md#path-operations` |
| 88 | + |
| 89 | +### 4. Dependency Injection - ABC Not Protocol 🔴 |
| 90 | + |
| 91 | +```python |
| 92 | +# ✅ CORRECT: Use ABC |
| 93 | +from abc import ABC, abstractmethod |
| 94 | + |
| 95 | +class MyOps(ABC): |
| 96 | + @abstractmethod |
| 97 | + def operation(self) -> None: ... |
| 98 | + |
| 99 | +# ❌ WRONG: Using Protocol |
| 100 | +from typing import Protocol |
| 101 | +``` |
| 102 | + |
| 103 | +**Details**: See `references/core-standards.md#dependency-injection` |
| 104 | + |
| 105 | +### 5. Imports - Module-Level and Absolute 🔴 |
| 106 | + |
| 107 | +**ALL imports must be at module level unless preventing circular imports** |
| 108 | + |
| 109 | +```python |
| 110 | +# ✅ CORRECT: Module-level, absolute imports |
| 111 | +from erk.config import load_config |
| 112 | +from pathlib import Path |
| 113 | +import click |
| 114 | + |
| 115 | +# ❌ WRONG: Inline imports (unless for circular import prevention) |
| 116 | +def my_function(): |
| 117 | + from erk.config import load_config # WRONG unless circular import |
| 118 | + return load_config() |
| 119 | + |
| 120 | +# ❌ WRONG: Relative imports |
| 121 | +from .config import load_config |
| 122 | +``` |
| 123 | + |
| 124 | +**Exception**: Inline imports are ONLY acceptable when preventing circular imports. Always document why: |
| 125 | + |
| 126 | +```python |
| 127 | +def create_context(): |
| 128 | + # Inline import to avoid circular dependency with tests |
| 129 | + from tests.fakes.gitops import FakeGitOps |
| 130 | + return FakeGitOps() |
| 131 | +``` |
| 132 | + |
| 133 | +**Details**: See `references/core-standards.md#imports` |
| 134 | + |
| 135 | +### 6. No Silent Fallback Behavior 🔴 |
| 136 | + |
| 137 | +```python |
| 138 | +# ❌ WRONG: Silent fallback |
| 139 | +try: |
| 140 | + result = primary_method() |
| 141 | +except: |
| 142 | + result = fallback_method() # Untested, brittle |
| 143 | + |
| 144 | +# ✅ CORRECT: Let error bubble up |
| 145 | +result = primary_method() |
| 146 | +``` |
| 147 | + |
| 148 | +**Details**: See `references/core-standards.md#anti-patterns` |
| 149 | + |
| 150 | +--- |
| 151 | + |
| 152 | +## When to Load References |
| 153 | + |
| 154 | +### Load `references/core-standards.md` when: |
| 155 | + |
| 156 | +- Writing exception handling code (LBYL patterns) |
| 157 | +- Working with type annotations (Python 3.13+ syntax) |
| 158 | +- Implementing path operations (exists() checks) |
| 159 | +- Creating ABC interfaces (dependency injection) |
| 160 | +- Organizing imports (absolute imports, module-level) |
| 161 | +- Working with CLI code (Click patterns) |
| 162 | +- Using dataclasses and immutability |
| 163 | +- Avoiding anti-patterns (silent fallback, exception swallowing) |
| 164 | +- Implementing `@property` or `__len__` (performance expectations) |
| 165 | + |
| 166 | +### Load `references/code-smells-dagster.md` when: |
| 167 | + |
| 168 | +- Designing function APIs (default parameters, keyword arguments) |
| 169 | +- Managing parameter complexity (parameter anxiety, invalid combinations) |
| 170 | +- Refactoring large functions/classes (god classes, local variables) |
| 171 | +- Working with context managers (assignment patterns) |
| 172 | +- Using `repr()` programmatically (string representation abuse) |
| 173 | +- Passing context objects (context coupling) |
| 174 | +- Dealing with error boundaries (early validation) |
| 175 | + |
| 176 | +### Load `references/patterns-reference.md` when: |
| 177 | + |
| 178 | +- Developing CLI commands with Click |
| 179 | +- Working with file I/O and pathlib |
| 180 | +- Implementing dataclasses and frozen structures |
| 181 | +- Managing subprocess operations |
| 182 | +- Reducing code nesting (early returns, helper functions) |
| 183 | + |
| 184 | +--- |
| 185 | + |
| 186 | +## Progressive Disclosure Guide |
| 187 | + |
| 188 | +This skill uses a three-level loading system: |
| 189 | + |
| 190 | +1. **This file (SKILL.md)**: Core rules and navigation (~350 lines) |
| 191 | +2. **Reference files**: Detailed patterns and examples (loaded as needed) |
| 192 | +3. **Quick lookup**: Use the tables above to find what you need |
| 193 | + |
| 194 | +The agent loads reference files only when needed based on the current task. The reference files contain: |
| 195 | + |
| 196 | +- **`core-standards.md`**: Foundational Python patterns from this skill |
| 197 | +- **`code-smells-dagster.md`**: Production-tested anti-patterns from Dagster Labs |
| 198 | +- **`patterns-reference.md`**: Common implementation patterns and examples |
| 199 | + |
| 200 | +--- |
| 201 | + |
| 202 | +## Philosophy |
| 203 | + |
| 204 | +**Write dignified Python code that:** |
| 205 | + |
| 206 | +- Fails fast at proper boundaries (not deep in the stack) |
| 207 | +- Makes invalid states unrepresentable (use the type system) |
| 208 | +- Expresses intent clearly (LBYL over EAFP) |
| 209 | +- Minimizes cognitive load (explicit over implicit) |
| 210 | +- Enables confident refactoring (test what you build) |
| 211 | + |
| 212 | +**Default stances:** |
| 213 | + |
| 214 | +- Let exceptions bubble up (handle at boundaries only) |
| 215 | +- Break APIs and migrate immediately (no unnecessary backwards compatibility) |
| 216 | +- Check conditions proactively (LBYL) |
| 217 | +- Use modern Python 3.13+ syntax |
| 218 | + |
| 219 | +--- |
| 220 | + |
| 221 | +## Quick Decision Tree |
| 222 | + |
| 223 | +**About to write Python code?** |
| 224 | + |
| 225 | +1. **Using `try/except`?** |
| 226 | + - Can you use LBYL instead? → Do that |
| 227 | + - Is this an error boundary? → OK to handle |
| 228 | + - Otherwise → Let it bubble |
| 229 | + |
| 230 | +2. **Using type hints?** |
| 231 | + - Use `list[str]`, `str | None`, not `List`, `Optional` |
| 232 | + - NO `from __future__ import annotations` |
| 233 | + |
| 234 | +3. **Working with paths?** |
| 235 | + - Check `.exists()` before `.resolve()` |
| 236 | + - Use `pathlib.Path`, not `os.path` |
| 237 | + |
| 238 | +4. **Writing CLI code?** |
| 239 | + - Use `click.echo()`, not `print()` |
| 240 | + - Exit with `raise SystemExit(1)` |
| 241 | + |
| 242 | +5. **Too many parameters?** |
| 243 | + - See `references/code-smells-dagster.md#parameter-anxiety` |
| 244 | + |
| 245 | +6. **Class getting large?** |
| 246 | + - See `references/code-smells-dagster.md#god-classes` |
| 247 | + |
| 248 | +--- |
| 249 | + |
| 250 | +## Checklist Before Writing Code |
| 251 | + |
| 252 | +Before writing `try/except`: |
| 253 | + |
| 254 | +- [ ] Can I check the condition proactively? (LBYL) |
| 255 | +- [ ] Is this at an error boundary? (CLI/API level) |
| 256 | +- [ ] Am I adding meaningful context or just hiding the error? |
| 257 | + |
| 258 | +Before using type hints: |
| 259 | + |
| 260 | +- [ ] Am I using Python 3.13+ syntax? (`list`, `dict`, `|`) |
| 261 | +- [ ] Have I removed all `typing` imports except essentials? |
| 262 | + |
| 263 | +Before path operations: |
| 264 | + |
| 265 | +- [ ] Did I check `.exists()` before `.resolve()`? |
| 266 | +- [ ] Am I using `pathlib.Path`? |
| 267 | +- [ ] Did I specify `encoding="utf-8"`? |
| 268 | + |
| 269 | +Before adding backwards compatibility: |
| 270 | + |
| 271 | +- [ ] Did the user explicitly request it? |
| 272 | +- [ ] Is this a public API? |
| 273 | +- [ ] Default: Break and migrate immediately |
| 274 | + |
| 275 | +--- |
| 276 | + |
| 277 | +## Common Patterns Summary |
| 278 | + |
| 279 | +| Scenario | Preferred Approach | Avoid | |
| 280 | +| --------------------- | ----------------------------------------- | ------------------------------------------- | |
| 281 | +| **Dictionary access** | `if key in dict:` or `.get(key, default)` | `try: dict[key] except KeyError:` | |
| 282 | +| **File existence** | `if path.exists():` | `try: open(path) except FileNotFoundError:` | |
| 283 | +| **Type checking** | `if isinstance(obj, Type):` | `try: obj.method() except AttributeError:` | |
| 284 | +| **Value validation** | `if is_valid(value):` | `try: process(value) except ValueError:` | |
| 285 | +| **Path resolution** | `if path.exists(): path.resolve()` | `try: path.resolve() except OSError:` | |
| 286 | + |
| 287 | +--- |
| 288 | + |
| 289 | +## References |
| 290 | + |
| 291 | +- **Core Standards**: `references/core-standards.md` - Detailed LBYL patterns, type annotations, imports |
| 292 | +- **Code Smells**: `references/code-smells-dagster.md` - Production-tested anti-patterns |
| 293 | +- **Pattern Reference**: `references/patterns-reference.md` - CLI, file I/O, dataclasses |
| 294 | +- Python 3.13 docs: https://docs.python.org/3.13/ |
0 commit comments