|
|
# Anti-Patterns
|
|
|
|
|
|
> ⚠️ **NOTE**: Section 1 below was **investigated and resolved** — it is NOT a bug (see §1 for details).
|
|
|
|
|
|
---
|
|
|
|
|
|
## 1. ~~CRITICAL: Entity-ID / Party-Name Mismatch in `compute_party_coords`~~ → **INVALID — INVESTIGATED & RESOLVED**
|
|
|
|
|
|
**Investigation Date**: 2026-03-31
|
|
|
|
|
|
**Investigation Summary**: After thorough analysis of the database schema and code, this anti-pattern is **INVALID**. The original concern was based on a false assumption about `svd_vectors.entity_id` containing party names.
|
|
|
|
|
|
**Investigation Findings**:
|
|
|
1. **`svd_vectors` table has NO rows with `entity_type='party'`** — only `mp` and `motion` entity types exist in practice.
|
|
|
2. **`entity_ids in svd_vectors are always MP names** (e.g., `"Van Dijk, I."`), never party names. The party centroids are correctly computed via `mp_metadata` lookups.
|
|
|
3. **The trajectories plot WORKS correctly** — no production bug exists. The code path for party-level visualization does not rely on `svd_vectors.entity_id` containing party names.
|
|
|
|
|
|
**Conclusion**: The original anti-pattern was a false positive caused by incorrect assumptions about data contents. The `party_map` reverse-lookup (`mp_name → party_name`) works correctly because `entity_id` values are always MP names, not party names.
|
|
|
|
|
|
---
|
|
|
|
|
|
## 2. Bare `except: pass`
|
|
|
|
|
|
**File**: `database.py`, line 47
|
|
|
|
|
|
**Problem**: Catches **all** exceptions including `KeyboardInterrupt`, `SystemExit`, `MemoryError`.
|
|
|
Silently swallows errors — no logging, no fallback.
|
|
|
|
|
|
**Broken code**:
|
|
|
```python
|
|
|
try:
|
|
|
self.conn.execute(sql)
|
|
|
except: # ← bare except
|
|
|
pass
|
|
|
```
|
|
|
|
|
|
**Fix**:
|
|
|
```python
|
|
|
try:
|
|
|
self.conn.execute(sql)
|
|
|
except ibis.errors.IbisError as e:
|
|
|
st.warning(f"Query failed: {e}")
|
|
|
raise # or return a default
|
|
|
```
|
|
|
|
|
|
---
|
|
|
|
|
|
## 3. Nested Exception Handling
|
|
|
|
|
|
**File**: `explorer.py`, lines 244–261
|
|
|
|
|
|
**Problem**: Try/except inside try/except creates opaque error paths. Inner exception silently swallows outer intent.
|
|
|
|
|
|
**Broken code**:
|
|
|
```python
|
|
|
try:
|
|
|
result = compute_svd(motions)
|
|
|
# ...
|
|
|
except Exception:
|
|
|
try:
|
|
|
# Try fallback approach
|
|
|
result = fallback_compute(motions)
|
|
|
except Exception:
|
|
|
pass # ← both exceptions silently dropped
|
|
|
```
|
|
|
|
|
|
**Fix**: Flatten — handle each case explicitly, or use a decorator.
|
|
|
|
|
|
---
|
|
|
|
|
|
## 4. Catch-All `Exception` Used Everywhere
|
|
|
|
|
|
**Problem**: `except Exception:` catches 50+ exception types including `ValueError`, `TypeError`, `KeyError`.
|
|
|
Overly broad — masks real bugs.
|
|
|
|
|
|
**Occurrence**: 850+ instances of bare/generic exception handlers across codebase.
|
|
|
|
|
|
**Fix**: Catch specific exceptions. If you must catch multiple, chain them:
|
|
|
```python
|
|
|
except (KeyError, ValueError) as e:
|
|
|
logger.warning(f"Missing field: {e}")
|
|
|
```
|
|
|
|
|
|
---
|
|
|
|
|
|
## 5. No `entity_id` Format Validation
|
|
|
|
|
|
**Problem**: `svd_vectors.entity_id` can be either:
|
|
|
- An MP name (e.g., `"Van Dijk, I."`) for individual-level SVD
|
|
|
- A party name (e.g., `"GroenLinks-PvdA"`) for party-level SVD
|
|
|
|
|
|
No validation distinguishes which is which. Code must infer from context. (Note: In practice `svd_vectors.entity_id` only contains MP names — see §1 for investigation findings.)
|
|
|
|
|
|
**Fix**: Add explicit format marker or separate columns:
|
|
|
```python
|
|
|
# Option A: separate columns
|
|
|
svd_vectors = pd.DataFrame({
|
|
|
'mp_name': [...], # nullable
|
|
|
'party_name': [...], # nullable
|
|
|
'window': [...],
|
|
|
'vector_2d': [...]
|
|
|
})
|
|
|
|
|
|
# Option B: format prefix
|
|
|
# "mp:Van Dijk, I." or "party:GroenLinks-PvdA"
|
|
|
```
|
|
|
|
|
|
---
|
|
|
|
|
|
## 6. Silent Fallback When Party Centroids Fail
|
|
|
|
|
|
**Problem**: If `party_map` lookup fails (entity is a party, not MP), the code silently produces
|
|
|
`party_map_count: 0` and empty `parties_with_centroid_counts`. No warning is raised.
|
|
|
|
|
|
**Fix**: Add validation and warning:
|
|
|
```python
|
|
|
if party_map_count == 0:
|
|
|
st.warning(f"No party mappings found for {len(svd_df)} entities in window '{window}'")
|
|
|
```
|
|
|
|
|
|
---
|
|
|
|
|
|
## 7. Three Separate Party Alias Dictionaries (No Single Source of Truth)
|
|
|
|
|
|
**Problem**: Party name variations exist in 3+ places:
|
|
|
- `PARTY_COLOURS` keys
|
|
|
- `party_map` values (from `mp_party_history`)
|
|
|
- Raw data column values
|
|
|
|
|
|
No canonical alias mapping. Spelling mismatches cause silent failures.
|
|
|
|
|
|
**Fix**: Create one `PARTY_ALIASES` dict in `config.py`:
|
|
|
```python
|
|
|
PARTY_ALIASES = {
|
|
|
"GroenLinks-PvdA": ["GL-PvdA", "GroenLinks PvdA", "PvdA-GroenLinks"],
|
|
|
"PVV": ["Partij voor de Vrijheid"],
|
|
|
...
|
|
|
}
|
|
|
|
|
|
def resolve_party(name: str) -> str:
|
|
|
"""Normalize any party name variant to canonical form."""
|
|
|
for canonical, aliases in PARTY_ALIASES.items():
|
|
|
if name in aliases or name == canonical:
|
|
|
return canonical
|
|
|
return name # no alias found
|
|
|
```
|
|
|
|
