refactor: delete stale files and consolidate .mindmodel structure
Deleted stale root-level Python files: - main.py (unused 'Hello world' script) - verify.py (unused table info script) - scraper.py (unused MotionScraper class) - scheduler.py (unused DataUpdateScheduler class) Deleted duplicate .mindmodel root YAML files (subdirectory versions are more comprehensive): - anti-patterns.yaml, architecture.yaml, conventions.yaml - dependencies.yaml, domain.yaml, domain-glossary.yaml - stack.yaml, tech-stack.yaml, workflows.yaml Added comprehensive .mindmodel subdirectories: - constraints/ (naming, db-schema, error-handling, types, etc.) - patterns/ (api, architecture, database, python, streamlit, etc.) - examples/ (code examples for each pattern) - anti-patterns/, architecture/, conventions/, dependencies/, domain/, stack/ Updated ARCHITECTURE.md to reflect current codebase: - Removed references to non-existent files - Added missing files (explorer.py, explorer_helpers.py, pipeline/) - Added directory structure documentation - Updated tech stack to include scipy, sklearn, umap Updated .gitignore: - Added patterns for generated analysis files - Added .worktrees/ pattern (was already in gitignore but dir was deleted) Removed empty .worktrees/ directory
This commit is contained in:
@@ -1,5 +1,51 @@
|
||||
# Mindmodel constraints README
|
||||
# Constraint Files Index
|
||||
|
||||
Files in .mindmodel/constraints/ are YAML-like constraint documents describing
|
||||
conventions, patterns and remediation steps. Use these to guide PR reviews and
|
||||
CI automation.
|
||||
This directory contains all constraint files for the Stemwijzer codebase.
|
||||
|
||||
## Quick Navigation
|
||||
|
||||
| Category | File | Purpose |
|
||||
|----------|------|---------|
|
||||
| **Stack** | `../stack/stack.yaml` | Tech stack overview |
|
||||
| **Architecture** | `../architecture/architecture.yaml` | Data flow, page routing, component relationships |
|
||||
| **Conventions** | `../conventions/conventions.yaml` | Naming, error handling, code organization |
|
||||
| **Domain** | `../domain/domain-glossary.yaml` | Dutch political terms, algorithm concepts |
|
||||
| **Patterns** | `../patterns/patterns.yaml` | 10 code patterns (page wrapper, pipeline, etc.) |
|
||||
| **Anti-Patterns** | `../anti-patterns/anti-patterns.yaml` | ⚠️ 7 issues including CRITICAL BUG |
|
||||
| **Dependencies** | `../dependencies/dependencies.yaml` | Library wiring, singletons, imports |
|
||||
|
||||
## How to Use
|
||||
|
||||
1. **Before writing code**: Check `patterns/patterns.yaml` for how similar features are implemented
|
||||
2. **When naming things**: Follow `conventions/conventions.yaml` (snake_case functions, PascalCase classes)
|
||||
3. **When handling errors**: Avoid patterns in `anti-patterns/anti-patterns.yaml`
|
||||
4. **When working with domain terms**: Reference `domain/domain-glossary.yaml`
|
||||
5. **When connecting components**: See `dependencies/dependencies.yaml` for wiring
|
||||
|
||||
## Key Conventions Summary
|
||||
|
||||
- **Files**: snake_case (`explorer_helpers.py`)
|
||||
- **Functions**: snake_case (`compute_party_coords`)
|
||||
- **Classes**: PascalCase (`MotionDatabase`)
|
||||
- **Constants**: UPPER_SNAKE_CASE (`PARTY_COLOURS`)
|
||||
- **No bare `except:`** — always specify exception type
|
||||
- **Pure functions** in helpers — no IO, no Streamlit calls
|
||||
- **One singleton per module** — `db`, `config`, `PARTY_COLOURS`
|
||||
|
||||
## ⚠️ Critical Bug
|
||||
|
||||
**Read `../anti-patterns/anti-patterns.yaml` first.** Section 1 documents a critical bug in
|
||||
`explorer_helpers.py:compute_party_coords` where party names in `svd_vectors` entity_id are
|
||||
not recognized because `party_map` only contains MP-name keys.
|
||||
|
||||
## Files Generated
|
||||
|
||||
- `manifest.yaml` — lists all constraint files with group mappings
|
||||
- `stack/stack.yaml` — tech stack
|
||||
- `architecture/architecture.yaml` — data flow & components
|
||||
- `conventions/conventions.yaml` — coding conventions
|
||||
- `domain/domain-glossary.yaml` — domain terminology
|
||||
- `patterns/patterns.yaml` — 10 code patterns with examples
|
||||
- `anti-patterns/anti-patterns.yaml` — 7 anti-patterns including CRITICAL BUG
|
||||
- `dependencies/dependencies.yaml` — library wiring
|
||||
- `README.md` — this index
|
||||
|
||||
@@ -0,0 +1,184 @@
|
||||
# Error Handling Constraints
|
||||
|
||||
## Core Rule
|
||||
|
||||
**Catch `Exception`, return safe fallbacks (False/[]/None)**
|
||||
|
||||
Never let exceptions propagate to user-facing code. Always provide a safe default.
|
||||
|
||||
## Patterns
|
||||
|
||||
### For Not-Found Operations
|
||||
|
||||
Return `None` or falsy value when item not found:
|
||||
|
||||
```python
|
||||
# GOOD: Return None on not found
|
||||
def get_motion_by_id(self, motion_id: int) -> Optional[Dict]:
|
||||
try:
|
||||
conn = duckdb.connect(self.db_path)
|
||||
result = conn.execute(
|
||||
"SELECT * FROM motions WHERE id = ?", (motion_id,)
|
||||
).fetchone()
|
||||
conn.close()
|
||||
return result
|
||||
except Exception:
|
||||
conn.close()
|
||||
return None
|
||||
```
|
||||
|
||||
### For Collection Operations
|
||||
|
||||
Return empty list when no results:
|
||||
|
||||
```python
|
||||
# GOOD: Return empty list on failure
|
||||
def get_filtered_motions(self, **kwargs) -> List[Dict]:
|
||||
try:
|
||||
conn = duckdb.connect(self.db_path)
|
||||
rows = conn.execute(query, params).fetchall()
|
||||
conn.close()
|
||||
return rows
|
||||
except Exception:
|
||||
conn.close()
|
||||
return []
|
||||
```
|
||||
|
||||
### For Boolean Operations
|
||||
|
||||
Return `False` for failed boolean checks:
|
||||
|
||||
```python
|
||||
# GOOD: Return False on failure
|
||||
def motion_exists(self, motion_id: int) -> bool:
|
||||
try:
|
||||
conn = duckdb.connect(self.db_path)
|
||||
count = conn.execute(
|
||||
"SELECT COUNT(*) FROM motions WHERE id = ?", (motion_id,)
|
||||
).fetchone()[0]
|
||||
conn.close()
|
||||
return count > 0
|
||||
except Exception:
|
||||
return False
|
||||
```
|
||||
|
||||
### For Creation Operations
|
||||
|
||||
Return `False` or empty string on failure:
|
||||
|
||||
```python
|
||||
# GOOD: Return empty string on failure
|
||||
def generate_summary(self, title: str, body: str) -> str:
|
||||
try:
|
||||
return ai_provider.chat_completion(messages)
|
||||
except ai_provider.ProviderError:
|
||||
logger.exception("AI provider failed")
|
||||
return ""
|
||||
```
|
||||
|
||||
## Anti-Patterns to Avoid
|
||||
|
||||
### Don't Catch Specific Exceptions Only
|
||||
```python
|
||||
# BAD: Catches only FileNotFoundError, misses other issues
|
||||
try:
|
||||
with open(path) as f:
|
||||
return json.load(f)
|
||||
except FileNotFoundError:
|
||||
return None
|
||||
```
|
||||
|
||||
### Don't Re-raise Without Context
|
||||
```python
|
||||
# BAD: Loses information
|
||||
try:
|
||||
process(data)
|
||||
except Exception:
|
||||
raise # No context added
|
||||
```
|
||||
|
||||
### Don't Swallow Exceptions Silently
|
||||
```python
|
||||
# BAD: No logging, no fallback
|
||||
try:
|
||||
return risky_operation()
|
||||
except Exception:
|
||||
pass # What happened?
|
||||
```
|
||||
|
||||
## Nested Exception Handling
|
||||
|
||||
When calling code that has its own error handling, wrap only if needed:
|
||||
|
||||
```python
|
||||
# Accept result from wrapped function (it handles errors)
|
||||
def fetch_motions(self, start_date):
|
||||
# ai_provider_wrapper handles retries internally
|
||||
embeddings = get_embeddings_with_retry(texts)
|
||||
|
||||
# Only wrap if wrapper doesn't handle errors
|
||||
if all(e is None for e in embeddings):
|
||||
logger.error("All embeddings failed")
|
||||
return []
|
||||
|
||||
return process(embeddings)
|
||||
```
|
||||
|
||||
## Context Managers
|
||||
|
||||
Use `try/finally` for cleanup:
|
||||
|
||||
```python
|
||||
def process_with_temp_file(self):
|
||||
temp = NamedTemporaryFile(delete=False)
|
||||
try:
|
||||
temp.write(data)
|
||||
temp.close()
|
||||
return process_file(temp.name)
|
||||
finally:
|
||||
os.unlink(temp.name)
|
||||
temp.close()
|
||||
```
|
||||
|
||||
## When to Log vs Return
|
||||
|
||||
| Scenario | Action |
|
||||
|----------|--------|
|
||||
| User action fails | Log warning, return safe default |
|
||||
| Internal error (corrupt data) | Log error, return safe default |
|
||||
| Transient failure (network) | Log warning, retry if appropriate |
|
||||
| Configuration error | Log error, raise with clear message |
|
||||
|
||||
## Exception Propagation
|
||||
|
||||
Only raise exceptions for:
|
||||
1. Configuration/setup errors (missing required env vars)
|
||||
2. Programming errors (invalid arguments)
|
||||
3. Fatal system errors (database corruption)
|
||||
|
||||
```python
|
||||
# GOOD: Raise for configuration errors
|
||||
def _get_api_key(self) -> str:
|
||||
key = os.environ.get("OPENROUTER_API_KEY")
|
||||
if not key:
|
||||
raise ProviderError(
|
||||
"OPENROUTER_API_KEY environment variable is required"
|
||||
)
|
||||
return key
|
||||
```
|
||||
|
||||
## Logging Errors
|
||||
|
||||
Always include context:
|
||||
|
||||
```python
|
||||
# GOOD: Include relevant context
|
||||
_logger.error(
|
||||
"Failed to fetch motion %d: %s",
|
||||
motion_id,
|
||||
exc
|
||||
)
|
||||
|
||||
# BAD: No context
|
||||
_logger.error("Failed to fetch")
|
||||
```
|
||||
@@ -1,24 +1,205 @@
|
||||
# Import grouping and ordering constraints
|
||||
# Import Organization Constraints
|
||||
|
||||
rules:
|
||||
- name: grouping
|
||||
rule: "Group imports in three sections separated by a single blank line: stdlib, third-party, local."
|
||||
examples:
|
||||
- good: |
|
||||
import json
|
||||
import logging
|
||||
## Standard Order
|
||||
|
||||
import requests
|
||||
import duckdb
|
||||
Organize imports in three groups with blank lines between:
|
||||
|
||||
from .pipeline import text_pipeline
|
||||
- bad: |
|
||||
import duckdb
|
||||
import json
|
||||
from pipeline import text_pipeline
|
||||
```python
|
||||
# 1. Standard library imports (alphabetical within group)
|
||||
import json
|
||||
import logging
|
||||
import os
|
||||
from datetime import datetime, timedelta
|
||||
from typing import Dict, List, Optional, Tuple
|
||||
|
||||
- name: from_imports
|
||||
rule: "Prefer 'from x import y' only when it improves clarity or avoids circular import; otherwise import module and reference attributes."
|
||||
# 2. Third-party packages (alphabetical within group)
|
||||
import duckdb
|
||||
import requests
|
||||
from config import config
|
||||
|
||||
enforcement_examples:
|
||||
- "Run isort or ruff- import sorting in pre-commit or CI to enforce ordering."
|
||||
# 3. Local application modules (can use relative imports)
|
||||
from database import db
|
||||
from summarizer import summarizer
|
||||
```
|
||||
|
||||
## Alphabetical Ordering
|
||||
|
||||
Within each group, sort imports alphabetically:
|
||||
|
||||
```python
|
||||
# GOOD - alphabetical
|
||||
import json
|
||||
import logging
|
||||
from datetime import datetime
|
||||
from typing import Dict, List, Optional
|
||||
|
||||
# BAD - random order
|
||||
from typing import Optional
|
||||
import json
|
||||
from datetime import datetime
|
||||
import logging
|
||||
from typing import Dict, List
|
||||
```
|
||||
|
||||
## Grouping Rules
|
||||
|
||||
### Standard Library
|
||||
- `json`, `logging`, `os`, `sys`, `time`
|
||||
- `datetime`, `timedelta` from `datetime`
|
||||
- `Dict`, `List`, `Optional`, etc. from `typing`
|
||||
- `argparse`, `pathlib`, `re`, `uuid`
|
||||
|
||||
### Third-Party
|
||||
- `duckdb`, `requests`, `streamlit`
|
||||
- `numpy`, `scipy`, `sklearn`
|
||||
- `plotly`, `beautifulsoup4`
|
||||
- `pytest`
|
||||
|
||||
### Local Application
|
||||
- Modules from same package
|
||||
- Relative imports when appropriate
|
||||
|
||||
## When to Use `from X import Y`
|
||||
|
||||
### Prefer `from module import specific_items` for:
|
||||
- Constants and config
|
||||
- Single classes or functions used frequently
|
||||
- Type annotations
|
||||
|
||||
```python
|
||||
# GOOD - clear about what we're using
|
||||
from config import config
|
||||
from database import db
|
||||
|
||||
# GOOD - type hints
|
||||
from typing import Dict, List, Optional
|
||||
```
|
||||
|
||||
### Use `import module` when:
|
||||
- You need multiple items from the module
|
||||
- Using module.namespace is clearer
|
||||
|
||||
```python
|
||||
# GOOD - duckdb used for types and module access
|
||||
import duckdb
|
||||
|
||||
conn = duckdb.connect(...)
|
||||
result = conn.execute(...)
|
||||
|
||||
# Also acceptable for types
|
||||
from typing import Dict
|
||||
```
|
||||
|
||||
## Relative Imports
|
||||
|
||||
In package modules, prefer relative imports:
|
||||
|
||||
```python
|
||||
# pipeline/svd_pipeline.py
|
||||
from ..database import MotionDatabase # relative import
|
||||
from .text_pipeline import process_text # relative import
|
||||
```
|
||||
|
||||
## Circular Imports
|
||||
|
||||
Avoid circular imports by:
|
||||
1. Moving shared code to a third module
|
||||
2. Using TYPE_CHECKING for type hints only
|
||||
|
||||
```python
|
||||
# types.py - shared type definitions
|
||||
from typing import TypedDict
|
||||
|
||||
class MotionDict(TypedDict):
|
||||
id: int
|
||||
title: str
|
||||
...
|
||||
|
||||
# module_a.py
|
||||
from .types import MotionDict
|
||||
|
||||
# module_b.py - if needed here too
|
||||
from .types import MotionDict
|
||||
```
|
||||
|
||||
## Import Patterns to Avoid
|
||||
|
||||
### Wildcard Imports
|
||||
```python
|
||||
# BAD
|
||||
from database import *
|
||||
|
||||
# GOOD
|
||||
from database import db, MotionDatabase
|
||||
```
|
||||
|
||||
### Import in Function Scope (unless necessary)
|
||||
```python
|
||||
# AVOID - delays import, makes dependencies unclear
|
||||
def some_function():
|
||||
import pandas as pd # Late import
|
||||
return pd.DataFrame(...)
|
||||
|
||||
# PREFER - import at module level
|
||||
import pandas as pd
|
||||
|
||||
def some_function():
|
||||
return pd.DataFrame(...)
|
||||
```
|
||||
|
||||
### Reassigning Imported Names
|
||||
```python
|
||||
# BAD - confusing
|
||||
from module import process
|
||||
process = something_else # Reassigning
|
||||
|
||||
# GOOD - clear naming
|
||||
from module import process as process_data
|
||||
```
|
||||
|
||||
## Type Checking Imports
|
||||
|
||||
For type hints only, use TYPE_CHECKING:
|
||||
|
||||
```python
|
||||
from typing import TYPE_CHECKING
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from .models import Motion
|
||||
|
||||
def get_motion(motion_id: int) -> "Motion": # String quote for forward ref
|
||||
...
|
||||
```
|
||||
|
||||
## Optional Dependency Imports
|
||||
|
||||
Handle optional dependencies gracefully:
|
||||
|
||||
```python
|
||||
try:
|
||||
import duckdb
|
||||
except Exception:
|
||||
duckdb = None # Will be checked later
|
||||
|
||||
class MotionDatabase:
|
||||
def __init__(self):
|
||||
if duckdb is None:
|
||||
self._file_mode = True # Fallback mode
|
||||
```
|
||||
|
||||
## Example: Complete Import Block
|
||||
|
||||
```python
|
||||
# Complete example from database.py
|
||||
import json
|
||||
import logging
|
||||
import uuid
|
||||
from datetime import datetime, timedelta
|
||||
from typing import Dict, List, Optional, Tuple
|
||||
|
||||
import duckdb
|
||||
|
||||
from config import config
|
||||
|
||||
from database import db
|
||||
```
|
||||
|
||||
@@ -0,0 +1,167 @@
|
||||
# Logging Constraints
|
||||
|
||||
## Core Rule
|
||||
|
||||
**Use `logging.getLogger(__name__)` - never use `print()`**
|
||||
|
||||
## Logger Initialization
|
||||
|
||||
Get logger at module level:
|
||||
|
||||
```python
|
||||
# GOOD: Use logging.getLogger(__name__)
|
||||
import logging
|
||||
|
||||
_logger = logging.getLogger(__name__)
|
||||
|
||||
def some_function():
|
||||
_logger.info("Processing started")
|
||||
_logger.debug("Detail: %s", detail)
|
||||
```
|
||||
|
||||
## Logger Naming
|
||||
|
||||
Use `__name__` for automatic module path:
|
||||
|
||||
```python
|
||||
# In database.py - logger will be "database"
|
||||
_logger = logging.getLogger(__name__)
|
||||
|
||||
# In pipeline/svd_pipeline.py - logger will be "pipeline.svd_pipeline"
|
||||
_logger = logging.getLogger(__name__)
|
||||
```
|
||||
|
||||
## Log Levels
|
||||
|
||||
| Level | When to Use |
|
||||
|-------|-------------|
|
||||
| DEBUG | Detailed diagnostic info (dev only) |
|
||||
| INFO | Normal operation milestones |
|
||||
| WARNING | Unexpected but handled (fallbacks) |
|
||||
| ERROR | Operation failed, may need attention |
|
||||
| CRITICAL | Fatal error, program may crash |
|
||||
|
||||
## Examples
|
||||
|
||||
### Good Logging Practice
|
||||
```python
|
||||
_logger.info("Pipeline run: %s → %s (%s windows)", start, end, count)
|
||||
_logger.debug("Batch embedding attempt %d failed: %s", attempt, exc)
|
||||
_logger.warning("Fallback used for motion %d: %s", motion_id, reason)
|
||||
_logger.error("Query failed: %s", exc)
|
||||
```
|
||||
|
||||
### Bad: Using print()
|
||||
```python
|
||||
# BAD - don't use print
|
||||
print(f"Fetched {len(voting_records)} voting records from API")
|
||||
print(f"Error fetching motions from API: {e}")
|
||||
```
|
||||
|
||||
### Good: Using logger
|
||||
```python
|
||||
# GOOD - use logger
|
||||
_logger.info("Fetched %d voting records from API", len(voting_records))
|
||||
_logger.error("Error fetching motions from API: %s", e)
|
||||
```
|
||||
|
||||
## Exception Logging
|
||||
|
||||
Use `_logger.exception()` for caught exceptions (includes traceback):
|
||||
|
||||
```python
|
||||
try:
|
||||
result = risky_operation()
|
||||
except Exception as exc:
|
||||
_logger.exception("Operation failed: %s", exc)
|
||||
return fallback_value
|
||||
```
|
||||
|
||||
Use `_logger.error()` with explicit exception for controlled errors:
|
||||
|
||||
```python
|
||||
try:
|
||||
result = risky_operation()
|
||||
except Exception as exc:
|
||||
_logger.error("Operation failed: %s", exc)
|
||||
return fallback_value
|
||||
```
|
||||
|
||||
## Configuration
|
||||
|
||||
Ensure logging is configured in entry points:
|
||||
|
||||
```python
|
||||
# pipeline/run_pipeline.py
|
||||
def run(args):
|
||||
logging.basicConfig(
|
||||
level=logging.INFO,
|
||||
format="%(asctime)s %(levelname)s %(name)s: %(message)s",
|
||||
)
|
||||
# ... rest of pipeline
|
||||
```
|
||||
|
||||
## Anti-Patterns
|
||||
|
||||
### Debug Prints in Production Code
|
||||
```python
|
||||
# BAD
|
||||
print(f"[TRAJ DEBUG] processing window {wid}")
|
||||
|
||||
# GOOD
|
||||
_logger.debug("Processing window %s", wid)
|
||||
```
|
||||
|
||||
### Inconsistent Logger Names
|
||||
```python
|
||||
# BAD - mixing _logger and logger
|
||||
_logger = logging.getLogger(__name__)
|
||||
logger = logging.getLogger("other") # Inconsistent
|
||||
|
||||
# GOOD - use single consistent pattern
|
||||
_logger = logging.getLogger(__name__)
|
||||
```
|
||||
|
||||
### Missing Logger Initialization
|
||||
```python
|
||||
# BAD - no logger defined
|
||||
def some_function():
|
||||
logging.getLogger(__name__).info("...") # Redundant calls
|
||||
|
||||
# GOOD - define once at module level
|
||||
_logger = logging.getLogger(__name__)
|
||||
|
||||
def some_function():
|
||||
_logger.info("...")
|
||||
```
|
||||
|
||||
## Sensitive Data
|
||||
|
||||
Never log sensitive information:
|
||||
- API keys
|
||||
- User votes
|
||||
- Session IDs (if tied to user data)
|
||||
- Personal information
|
||||
|
||||
```python
|
||||
# BAD
|
||||
_logger.info("User %s voted %s", user_id, vote)
|
||||
|
||||
# GOOD - log aggregates, not individual votes
|
||||
_logger.info("Vote recorded for session %s", session_id[:8])
|
||||
```
|
||||
|
||||
## Structured Logging
|
||||
|
||||
For complex data, use structured logging:
|
||||
|
||||
```python
|
||||
_logger.info(
|
||||
"Motion processed",
|
||||
extra={
|
||||
"motion_id": motion_id,
|
||||
"policy_area": policy_area,
|
||||
"processing_time_ms": elapsed_ms,
|
||||
}
|
||||
)
|
||||
```
|
||||
@@ -1,30 +1,141 @@
|
||||
# Naming constraint rules (example constraint file)
|
||||
# Naming Constraints
|
||||
|
||||
rules:
|
||||
- name: module_file_names
|
||||
rule: "Use snake_case for Python module filenames (e.g., text_pipeline.py, ai_provider.py)."
|
||||
examples:
|
||||
- good: "text_pipeline.py"
|
||||
- bad: "TextPipeline.py"
|
||||
## File Names
|
||||
|
||||
- name: function_names
|
||||
rule: "Use snake_case for functions and methods."
|
||||
examples:
|
||||
- good: "def compute_similarities(...):"
|
||||
- bad: "def ComputeSimilarities(...):"
|
||||
### Python Modules
|
||||
- **Convention**: `snake_case.py`
|
||||
- **Examples**: `motion_database.py`, `api_client.py`, `text_pipeline.py`
|
||||
|
||||
- name: class_names
|
||||
rule: "Use PascalCase for classes."
|
||||
examples:
|
||||
- good: "class MotionDatabase:"
|
||||
- bad: "class motion_database:"
|
||||
### Test Files
|
||||
- **Convention**: `test_<module_name>.py`
|
||||
- **Examples**: `test_database.py`, `test_api_client.py`
|
||||
|
||||
- name: constants
|
||||
rule: "Constants use UPPER_SNAKE_CASE."
|
||||
examples:
|
||||
- good: "VOTE_MAP = { ... }"
|
||||
- bad: "vote_map = { ... }"
|
||||
### Config Files
|
||||
- **Convention**: `snake_case`
|
||||
- **Examples**: `config.py`, `.env.example`, `pyproject.toml`
|
||||
|
||||
enforcement_examples:
|
||||
- "Add a linter rule in CI: ruff or flake8 naming plugin to detect violations."
|
||||
- "Run `python -m pip install ruff` and `ruff check` as part of CI."
|
||||
### Directories
|
||||
- **Convention**: `snake_case/`
|
||||
- **Examples**: `pipeline/`, `tests/integration/`, `src/validators/`
|
||||
|
||||
## Class Names
|
||||
|
||||
- **Convention**: `PascalCase`
|
||||
- **Examples**: `MotionDatabase`, `TweedeKamerAPI`, `MotionSummarizer`
|
||||
|
||||
### Naming Patterns
|
||||
| Pattern | Example |
|
||||
|---------|---------|
|
||||
| Database wrapper | `MotionDatabase` |
|
||||
| API client | `TweedeKamerAPI` |
|
||||
| Service/Helpers | `MotionScraper`, `MotionAnalyzer` |
|
||||
| Exceptions | `ProviderError` |
|
||||
|
||||
## Function Names
|
||||
|
||||
- **Convention**: `snake_case`
|
||||
- **Examples**: `get_motions`, `compute_similarity`, `process_voting_records`
|
||||
|
||||
### Private Methods
|
||||
- **Convention**: `_snake_case` (single underscore prefix)
|
||||
- **Examples**: `_get_voting_records`, `_parse_response`
|
||||
|
||||
## Variable Names
|
||||
|
||||
### Regular Variables
|
||||
- **Convention**: `snake_case`
|
||||
- **Examples**: `motion_id`, `party_name`, `voting_results`
|
||||
|
||||
### Constants (Module-Level)
|
||||
- **Convention**: `UPPER_SNAKE_CASE`
|
||||
- **Examples**: `DATABASE_PATH`, `API_TIMEOUT`, `MAX_RETRIES`
|
||||
|
||||
### Config Variables (in dataclass)
|
||||
- **Convention**: `UPPER_SNAKE_CASE`
|
||||
- **Examples**: `QWEN_MODEL`, `POLICY_AREAS`
|
||||
|
||||
### Booleans
|
||||
- **Convention**: `is_`, `has_`, `can_` prefixes or `_flag` suffix
|
||||
- **Examples**: `is_active`, `has_votes`, `skip_extract`
|
||||
|
||||
### Private Variables
|
||||
- **Convention**: `_underscore_prefix`
|
||||
- **Examples**: `_conn`, `_cache`, `_session`
|
||||
|
||||
## Singleton Instances
|
||||
|
||||
- **Convention**: `lower_snake_case` at module level
|
||||
- **Examples**: `db = MotionDatabase()`, `summarizer = MotionSummarizer()`
|
||||
|
||||
```python
|
||||
# database.py
|
||||
class MotionDatabase:
|
||||
...
|
||||
|
||||
# Singleton instance
|
||||
db = MotionDatabase()
|
||||
|
||||
# Usage
|
||||
from database import db
|
||||
motions = db.get_motions()
|
||||
```
|
||||
|
||||
## Type Variables
|
||||
|
||||
- **Convention**: `PascalCase`
|
||||
- **Examples**: `T = TypeVar('T')`, `MotionDict = Dict[str, Any]`
|
||||
|
||||
## Anti-Patterns
|
||||
|
||||
### Inconsistent Naming
|
||||
```python
|
||||
# BAD - mixing styles
|
||||
get_motions() # snake_case
|
||||
GetMotionById() # PascalCase
|
||||
processData() # camelCase
|
||||
|
||||
# GOOD - consistent snake_case
|
||||
get_motions()
|
||||
get_motion_by_id()
|
||||
process_voting_data()
|
||||
```
|
||||
|
||||
### Abbreviations
|
||||
```python
|
||||
# AVOID - unclear abbreviations
|
||||
calc_similarity() # calculate_*
|
||||
proc_votes() # process_*
|
||||
get_mp_data() # get_mp_metadata()
|
||||
|
||||
# PREFER - full words
|
||||
calculate_similarity()
|
||||
process_votes()
|
||||
get_mp_metadata()
|
||||
```
|
||||
|
||||
### Hungarian Notation
|
||||
```python
|
||||
# BAD - Hungarian notation
|
||||
str_title = "..."
|
||||
int_count = 0
|
||||
b_is_active = True
|
||||
|
||||
# GOOD - clear types via naming
|
||||
title = "..."
|
||||
count = 0
|
||||
is_active = True
|
||||
```
|
||||
|
||||
## Special Cases
|
||||
|
||||
### Window IDs
|
||||
- **Format**: `"YYYY-QN"` or `"YYYY"`
|
||||
- **Examples**: `"2024-Q1"`, `"2024-Q2"`, `"2024"`
|
||||
|
||||
### Policy Areas
|
||||
- **Convention**: PascalCase with spaces
|
||||
- **Examples**: `"Economie"`, `"Sociale Zaken"`, `"Klimaat"`
|
||||
|
||||
### Vote Values
|
||||
- **Convention**: PascalCase Dutch terms
|
||||
- **Values**: `"Voor"`, `"Tegen"`, `"Onthouden"`, `"Geen stem"`, `"Afwezig"`
|
||||
|
||||
@@ -0,0 +1,233 @@
|
||||
# Type Hint Constraints
|
||||
|
||||
## Core Rule
|
||||
|
||||
**Use type hints on all public functions and methods**
|
||||
|
||||
## Function Type Hints
|
||||
|
||||
### Required on Public APIs
|
||||
|
||||
```python
|
||||
# GOOD - complete type hints
|
||||
def get_motion(self, motion_id: int) -> Optional[Dict]:
|
||||
...
|
||||
|
||||
def get_filtered_motions(
|
||||
self,
|
||||
policy_area: str = "Alle",
|
||||
limit: int = 10
|
||||
) -> List[Dict]:
|
||||
...
|
||||
|
||||
def calculate_similarity(self, motion_a: int, motion_b: int) -> float:
|
||||
...
|
||||
```
|
||||
|
||||
### Optional Parameters
|
||||
|
||||
Use `Optional[X]` or `X | None`:
|
||||
|
||||
```python
|
||||
# Both forms are acceptable
|
||||
def get_motion(self, motion_id: Optional[int] = None) -> Optional[Dict]:
|
||||
...
|
||||
|
||||
def get_motion(self, motion_id: int | None = None) -> dict | None:
|
||||
...
|
||||
```
|
||||
|
||||
### Multiple Return Types
|
||||
|
||||
Use `Union[X, Y]` or `|` operator:
|
||||
|
||||
```python
|
||||
# Acceptable forms
|
||||
def parse_value(self, value: str) -> Union[bool, str, None]:
|
||||
...
|
||||
|
||||
def parse_value(self, value: str) -> bool | str | None:
|
||||
...
|
||||
```
|
||||
|
||||
### Generic Types
|
||||
|
||||
Use `List[X]`, `Dict[K, V]`, `Tuple[X, Y]`:
|
||||
|
||||
```python
|
||||
from typing import Dict, List, Optional, Tuple
|
||||
|
||||
def get_motions(self, ids: List[int]) -> Dict[int, Dict]:
|
||||
"""Map motion_id -> motion data."""
|
||||
...
|
||||
|
||||
def process_batch(self, items: List[str]) -> Tuple[List[str], List[str]]:
|
||||
"""Returns (successes, failures)."""
|
||||
...
|
||||
```
|
||||
|
||||
## Collection Types
|
||||
|
||||
Prefer specific types over bare `list`/`dict`:
|
||||
|
||||
```python
|
||||
# GOOD - specific types
|
||||
def get_votes(self) -> List[str]:
|
||||
...
|
||||
|
||||
def get_metadata(self) -> Dict[str, Any]:
|
||||
...
|
||||
|
||||
# ACCEPTABLE - for truly generic collections
|
||||
def merge_dicts(*dicts: dict) -> dict:
|
||||
...
|
||||
```
|
||||
|
||||
## DuckDB Result Types
|
||||
|
||||
DuckDB returns tuples/lists - document expected structure:
|
||||
|
||||
```python
|
||||
def get_motion(self, motion_id: int) -> Optional[Tuple]:
|
||||
"""Returns (id, title, description, date, ...) or None."""
|
||||
conn = duckdb.connect(self.db_path)
|
||||
try:
|
||||
result = conn.execute(
|
||||
"SELECT * FROM motions WHERE id = ?", (motion_id,)
|
||||
).fetchone()
|
||||
return result
|
||||
finally:
|
||||
conn.close()
|
||||
|
||||
# Or use Dict for clarity
|
||||
def get_motion_as_dict(self, motion_id: int) -> Optional[Dict]:
|
||||
"""Returns motion dict or None."""
|
||||
conn = duckdb.connect(self.db_path)
|
||||
try:
|
||||
row = conn.execute(
|
||||
"SELECT * FROM motions WHERE id = ?", (motion_id,)
|
||||
).fetchone()
|
||||
if row:
|
||||
return {
|
||||
"id": row[0],
|
||||
"title": row[1],
|
||||
"description": row[2],
|
||||
...
|
||||
}
|
||||
return None
|
||||
finally:
|
||||
conn.close()
|
||||
```
|
||||
|
||||
## Class/Instance Types
|
||||
|
||||
Use `Self` for methods returning instance type:
|
||||
|
||||
```python
|
||||
from typing import Self
|
||||
|
||||
class MotionDatabase:
|
||||
def with_connection(self, path: str) -> Self:
|
||||
"""Return new instance with different path."""
|
||||
return MotionDatabase(db_path=path)
|
||||
```
|
||||
|
||||
## Callback/Function Types
|
||||
|
||||
Use `Callable` for function parameters:
|
||||
|
||||
```python
|
||||
from typing import Callable
|
||||
|
||||
def process_motions(
|
||||
motions: List[Dict],
|
||||
processor: Callable[[Dict], Any]
|
||||
) -> List[Any]:
|
||||
return [processor(m) for m in motions]
|
||||
```
|
||||
|
||||
## Type Aliases
|
||||
|
||||
Define clear type aliases for domain concepts:
|
||||
|
||||
```python
|
||||
from typing import Dict, List, TypedDict, Literal
|
||||
|
||||
# Vote values
|
||||
VoteValue = Literal["Voor", "Tegen", "Onthouden", "Geen stem", "Afwezig"]
|
||||
|
||||
# Policy areas
|
||||
PolicyArea = Literal["Alle", "Economie", "Klimaat", "Immigratie", ...]
|
||||
|
||||
# Motion dict
|
||||
class MotionDict(TypedDict):
|
||||
id: int
|
||||
title: str
|
||||
description: Optional[str]
|
||||
date: Optional[str]
|
||||
policy_area: Optional[str]
|
||||
voting_results: Optional[str] # JSON string
|
||||
winning_margin: Optional[float]
|
||||
|
||||
def get_motion(self, motion_id: int) -> Optional[MotionDict]:
|
||||
...
|
||||
```
|
||||
|
||||
## Avoid `Any`
|
||||
|
||||
Use `Any` sparingly - prefer specific types:
|
||||
|
||||
```python
|
||||
# AVOID - too vague
|
||||
def process(data: Any) -> Any:
|
||||
...
|
||||
|
||||
# PREFER - specific types
|
||||
def process(motion: MotionDict) -> Optional[SimilarityResult]:
|
||||
...
|
||||
```
|
||||
|
||||
## Inline Type Hints
|
||||
|
||||
For simple cases, inline hints are fine:
|
||||
|
||||
```python
|
||||
def get_count(self) -> int:
|
||||
...
|
||||
|
||||
def is_empty(self) -> bool:
|
||||
...
|
||||
```
|
||||
|
||||
## Docstring Type Hints
|
||||
|
||||
For complex types, include in docstrings:
|
||||
|
||||
```python
|
||||
def get_party_positions(self, window_id: str) -> Dict[str, List[float]]:
|
||||
"""Get party positions in political space.
|
||||
|
||||
Args:
|
||||
window_id: Time window (e.g., "2024-Q1")
|
||||
|
||||
Returns:
|
||||
Dict mapping party_name -> [x, y] coordinates
|
||||
|
||||
Example:
|
||||
>>> positions = db.get_party_positions("2024-Q1")
|
||||
>>> positions["VVD"]
|
||||
[0.5, -0.3]
|
||||
"""
|
||||
...
|
||||
```
|
||||
|
||||
## Type Checking
|
||||
|
||||
For runtime type checking, use runtime checks:
|
||||
|
||||
```python
|
||||
def set_count(self, count: int) -> None:
|
||||
if not isinstance(count, int):
|
||||
raise TypeError(f"Expected int, got {type(count).__name__}")
|
||||
self._count = count
|
||||
```
|
||||
Reference in New Issue
Block a user