simplerick0/python-quality-review

Python Quality Review

Review Python code for idiomatic patterns, correctness, and best practices.

Python-Specific Checks

Type Hints

# Missing type hints
def process(data):
    return data.get('value')

# With type hints
def process(data: dict[str, Any]) -> str | None:
    return data.get('value')

Mutable Default Arguments

# Bug: Mutable default (shared across calls)
def append_to(item, target=[]):
    target.append(item)
    return target

# Fixed: Use None default
def append_to(item, target: list | None = None):
    if target is None:
        target = []
    target.append(item)
    return target

Exception Handling

# Too broad
try:
    result = process(data)
except Exception:
    pass  # Silently swallows all errors

# Better: Specific exceptions
try:
    result = process(data)
except ValidationError as e:
    logger.warning(f"Validation failed: {e}")
    return None
except ConnectionError as e:
    logger.error(f"Connection failed: {e}")
    raise

Pythonic Patterns

Use Comprehensions

# Non-Pythonic
result = []
for item in items:
    if item.is_valid:
        result.append(item.value)

# Pythonic
result = [item.value for item in items if item.is_valid]

Use Context Managers

# Bad: Manual resource management
f = open('file.txt')
try:
    data = f.read()
finally:
    f.close()

# Good: Context manager
with open('file.txt') as f:
    data = f.read()

Use Unpacking

# Verbose
first = items[0]
second = items[1]
rest = items[2:]

# Pythonic
first, second, *rest = items

Use enumerate/zip

# Index tracking manually
i = 0
for item in items:
    print(f"{i}: {item}")
    i += 1

# Use enumerate
for i, item in enumerate(items):
    print(f"{i}: {item}")

Async/Await Issues

Blocking in Async

# Bug: Blocking call in async function
async def fetch_data():
    response = requests.get(url)  # Blocks the event loop!
    return response.json()

# Fixed: Use async library
async def fetch_data():
    async with aiohttp.ClientSession() as session:
        async with session.get(url) as response:
            return await response.json()

Missing Await

# Bug: Coroutine never awaited
async def process():
    fetch_data()  # Returns coroutine, doesn't execute!

# Fixed
async def process():
    await fetch_data()

Concurrent Execution

# Sequential (slow)
async def fetch_all(urls):
    results = []
    for url in urls:
        results.append(await fetch(url))
    return results

# Concurrent (fast)
async def fetch_all(urls):
    return await asyncio.gather(*[fetch(url) for url in urls])

Performance Patterns

String Concatenation

# Slow: String concatenation in loop
result = ""
for item in items:
    result += str(item)

# Fast: Join
result = "".join(str(item) for item in items)

Generator vs List

# Memory heavy: Creates full list
def get_values(items):
    return [expensive_compute(item) for item in items]

# Memory efficient: Generator
def get_values(items):
    return (expensive_compute(item) for item in items)

Dict/Set for Lookups

# Slow: O(n) lookup
if item in large_list:
    ...

# Fast: O(1) lookup
large_set = set(large_list)
if item in large_set:
    ...

Review Checklist

Correctness

• [ ] No mutable default arguments
• [ ] Exceptions caught specifically (not bare except)
• [ ] Resources properly closed (context managers)
• [ ] Async/await used correctly

Style

• [ ] Type hints on public functions
• [ ] Docstrings on public functions
• [ ] Comprehensions where appropriate
• [ ] Pythonic idioms used

Performance

• [ ] No O(n²) operations in loops
• [ ] Generators for large sequences
• [ ] Set/dict for membership testing
• [ ] No blocking calls in async code

Review Output Format

## Python Review: [File Name]

### Type Safety
- [ ] Type hints complete
- [ ] Types are accurate
- [ ] Optional types handled

### Python Issues
- **Line X**: [Issue description]
  ```python
  # Current
  problematic_code()

  # Suggested
  better_code()

Async Issues

• [List any async/await problems]

Performance Concerns

• [List any performance issues]