python-code-review

What this skill does

# Python Code Review Skill

A specialized skill for reviewing Python code with focus on PEP8 compliance, type hints, modern best practices, and code quality.

## Overview

This skill enables Claude to autonomously review Python code for compliance with modern Python standards, including PEP8 style guidelines, type hint coverage, proper structure, and common anti-patterns. It promotes clean, maintainable, and type-safe Python code.

## Capabilities

When activated, this skill provides:

1. **PEP8 Compliance Review**
   - Check naming conventions (snake_case, PascalCase)
   - Verify line length (88 characters)
   - Review whitespace and indentation
   - Check import ordering and grouping
   - Validate docstring presence and format

2. **Type Hints Validation**
   - Ensure all functions have type hints
   - Check for proper return type annotations
   - Validate use of modern type syntax (list[], dict[] over List[], Dict[])
   - Verify Optional[] vs None union types
   - Check for Any types (discourage overuse)

3. **Code Structure & Organization**
   - Review module structure and imports
   - Check for proper `__init__.py` usage
   - Validate entry points (`__main__.py`)
   - Review function/class organization
   - Assess module coupling and cohesion

4. **Best Practices Enforcement**
   - Check for use of context managers (with statements)
   - Validate exception handling
   - Review string formatting (f-strings preferred)
   - Check for proper constant definitions
   - Assess use of comprehensions vs loops

5. **Common Anti-Patterns Detection**
   - Mutable default arguments
   - Bare except clauses
   - Using `global` keyword unnecessarily
   - Poor variable naming
   - Overly complex functions (high cyclomatic complexity)

6. **Documentation Quality**
   - Check for docstrings on public functions/classes
   - Validate docstring format (Google/NumPy style)
   - Review parameter documentation
   - Check for return value documentation
   - Assess example usage in docstrings

## Usage

This skill activates automatically when:
- User requests: "Review this Python code"
- User asks: "Check PEP8 compliance"
- User mentions: "Validate type hints"
- User wants: "Improve this Python code"
- Code review is requested for .py files
- User asks: "Is this code following best practices?"

## Review Approach

### 1. Initial Assessment
- Identify Python version being used
- Understand the code's purpose
- Determine if it's application code or library code
- Check project structure context

### 2. PEP8 Compliance Check

**Naming Conventions:**
- ✅ Functions/variables: `snake_case`
- ✅ Classes: `PascalCase`
- ✅ Constants: `UPPER_SNAKE_CASE`
- ✅ Private members: `_leading_underscore`
- ✅ Module names: `lowercase` or `snake_case`

**Code Layout:**
- ✅ Line length: 88 characters (Black standard)
- ✅ Indentation: 4 spaces
- ✅ Blank lines: 2 before top-level definitions, 1 between methods
- ✅ Import order: stdlib → third-party → local
- ✅ One import per line (except from imports)

### 3. Type Hints Review

**Check for:**
```python
# ❌ Missing type hints
def process(data):
    return data.upper()

# ✅ Proper type hints
def process(data: str) -> str:
    return data.upper()

# ❌ Old-style typing (Python <3.9)
from typing import List, Dict
def get_items() -> List[Dict[str, int]]:
    pass

# ✅ Modern type syntax (Python 3.9+)
def get_items() -> list[dict[str, int]]:
    pass

# ❌ Overuse of Any
from typing import Any
def process(data: Any) -> Any:
    pass

# ✅ Specific types
def process(data: str | int) -> str:
    return str(data)
```

### 4. Common Issues Detection

**Mutable Default Arguments:**
```python
# ❌ Dangerous
def append_to(element: str, target: list[str] = []) -> list[str]:
    target.append(element)
    return target

# ✅ Safe
def append_to(element: str, target: list[str] | None = None) -> list[str]:
    if target is None:
        target = []
    target.append(element)
    return target
```

**Exception Handling:**
```python
# ❌ Bare except
try:
    risky_operation()
except:
    pass

# ✅ Specific exception
try:
    risky_operation()
except ValueError as e:
    logger.error(f"Invalid value: {e}")
    raise
```

**String Formatting:**
```python
# ❌ Old style
name = "World"
print("Hello %s" % name)
print("Hello {}".format(name))

# ✅ Modern f-strings
print(f"Hello {name}")
```

**Context Managers:**
```python
# ❌ Manual resource management
file = open("data.txt")
data = file.read()
file.close()

# ✅ Context manager
with open("data.txt") as file:
    data = file.read()
```

### 5. Code Quality Assessment

**Function Complexity:**
- Functions should do one thing well
- Aim for <20 lines per function
- Cyclomatic complexity should be low (<10)
- Deep nesting (>3 levels) indicates need for refactoring

**Example - Complex to Simple:**
```python
# ❌ Too complex
def process_user(user):
    if user:
        if user.active:
            if user.email:
                if "@" in user.email:
                    return user.email.lower()
    return None

# ✅ Simplified
def process_user(user: User | None) -> str | None:
    if not user or not user.active or not user.email:
        return None
    
    if "@" not in user.email:
        return None
        
    return user.email.lower()
```

## Output Format

Provide review results in this structured format:

```markdown
## Python Code Review Results

### Summary
[Brief overview of the code quality and main findings]

### PEP8 Compliance: [PASS/FAIL/PARTIAL]
**Issues Found:**
- Line 15: Line too long (102 characters, limit 88)
- Line 23: Missing blank line after function definition
- Line 45: Import should be at top of file

### Type Hints Coverage: [X%]
**Missing Type Hints:**
- Line 10: Function `process_data` missing return type
- Line 18: Parameter `config` missing type annotation
- Line 25: Using deprecated `List` instead of `list`

**Recommendations:**
```python
# Current
def process_data(items, limit=10):
    return items[:limit]

# Improved
def process_data(items: list[str], limit: int = 10) -> list[str]:
    return items[:limit]
```

### Code Quality Issues

#### Critical 🔴
- **Line 34**: Mutable default argument in `add_item(item, items=[])`
- **Line 56**: Bare except clause catching all exceptions

#### Important 🟡
- **Line 12**: Function complexity too high (15 branches)
- **Line 78**: Using `global` keyword - consider refactoring
- **Line 91**: Variable name `x` is not descriptive

#### Minor 🔵
- **Line 23**: Could use f-string instead of .format()
- **Line 45**: List comprehension would be more Pythonic

### Best Practices Violations

1. **Line 15-18: Manual file handling**
   - **Current:**
   ```python
   f = open("data.txt")
   data = f.read()
   f.close()
   ```
   - **Improved:**
   ```python
   with open("data.txt") as f:
       data = f.read()
   ```

2. **Line 34: Mutable default argument**
   - **Impact**: Can cause bugs when function is called multiple times
   - **Fix**: Use None as default, create new list inside function

### Positive Aspects ✅
- Consistent naming conventions throughout
- Good use of type hints in 80% of functions
- Proper docstrings on all public methods
- Clean import organization

### Documentation Review
- ✅ All public functions have docstrings
- ✅ Docstrings follow Google style format
- ⚠️ Some docstrings missing return value description
- ⚠️ No examples in docstrings (consider adding for complex functions)

### Modernization Suggestions
- Update to Python 3.10+ syntax (use `|` for unions instead of `Union`)
- Consider using `match/case` statement (Python 3.10+) for complex conditionals
- Replace `Dict`, `List` with `dict`, `list` (Python 3.9+)

### Next Steps
1. Fix critical issues (mutable defaults, bare excepts)
2. Add missing type hints
3. Refactor complex functions (>15 lines)
4. Run automated tools:
   ```bash
   uv run ruff check .          # Linting
   uv run mypy .                # Type checking
   uv run ruff format .         # Auto-formatting
   ```

### Recommende