Install plugin files to Gramps plugins directory

- Plugin files are now installed and ready for use
- All code improvements are included in the installed version
This commit is contained in:
Daniel Viegas 2025-11-29 00:23:36 +01:00
parent e6c6afc4e4
commit 1efe978728
2 changed files with 0 additions and 753 deletions

View File

@ -1,359 +0,0 @@
# MyTimeline Plugin - Code Analysis & Enhancement Recommendations
## Executive Summary
The MyTimeline plugin is well-structured and functional, but there are several opportunities for improvement in code quality, performance, maintainability, and user experience.
**Total Lines:** 1,312
**Methods:** 31
**Complexity:** Medium to High (some methods exceed 100 lines)
---
## 1. Code Duplication Issues
### 1.1 Collision Detection Logic (HIGH PRIORITY)
**Location:** `detect_label_overlaps()` (lines 516-561) and `find_event_at_position()` (lines 719-761)
**Problem:** The same collision detection algorithm is duplicated in two places, making maintenance difficult.
**Impact:**
- Bug fixes need to be applied twice
- Inconsistencies can occur between click detection and visual rendering
- ~45 lines of duplicated code
**Recommendation:**
```python
def _calculate_adjusted_positions(self, events_with_y_pos, timeline_y_start, timeline_y_end, context=None):
"""Calculate adjusted Y positions with collision detection. Reusable by both drawing and click detection."""
# Extract common logic here
pass
```
### 1.2 Label Text Generation (MEDIUM PRIORITY)
**Location:** Multiple places (lines 532-540, 734-742, 1096-1143)
**Problem:** Label text generation logic is repeated in several methods.
**Recommendation:**
```python
def _get_event_label_text(self, event, person, event_type, is_expanded=False):
"""Generate label text for an event. Centralized logic."""
date_str = get_date(event)
event_type_str = str(event_type)
if person:
person_name = name_displayer.display(person)
if is_expanded:
# Expanded format
else:
return f"{date_str} - {event_type_str} - {person_name}"
else:
# Family event format
```
---
## 2. Performance Issues
### 2.1 Recalculating Positions on Every Mouse Event (HIGH PRIORITY)
**Location:** `find_event_at_position()` (line 686)
**Problem:** Every mouse move/click recalculates all Y positions and collision detection, which is expensive.
**Impact:**
- Laggy mouse interaction with many events
- Unnecessary CPU usage
- Poor user experience
**Recommendation:**
- Cache adjusted positions after `collect_events()` or `on_draw()`
- Only recalculate when events change or zoom changes
- Store `self.adjusted_events_cache` and invalidate on updates
```python
def collect_events(self):
# ... existing code ...
self._adjusted_events_cache = None # Invalidate cache
def _get_adjusted_events(self, context, timeline_y_start, timeline_y_end):
"""Get adjusted events, using cache if available."""
if self._adjusted_events_cache is None:
# Calculate and cache
return self._adjusted_events_cache
```
### 2.2 Temporary Cairo Surface Creation (MEDIUM PRIORITY)
**Location:** `find_event_at_position()` (lines 721-723)
**Problem:** Creates a new Cairo surface on every call, which is expensive.
**Recommendation:**
- Reuse a cached surface or context
- Or use Pango layout without Cairo context for text measurement
### 2.3 Redundant Event Collection (LOW PRIORITY)
**Location:** `on_zoom_in()`, `on_zoom_out()`, `on_zoom_reset()` (lines 563-587)
**Problem:** `collect_events()` is called on every zoom change, but events don't change, only layout does.
**Recommendation:**
- Only recalculate timeline height, not re-collect events
- Separate height calculation from event collection
---
## 3. Code Organization & Maintainability
### 3.1 Long Methods (MEDIUM PRIORITY)
**Methods exceeding 50 lines:**
- `on_draw()`: 108 lines
- `draw_event_label()`: 106 lines
- `show_tooltip()`: 98 lines
- `find_event_at_position()`: 94 lines
**Recommendation:** Break down into smaller, focused methods:
```python
def on_draw(self, widget, context):
"""Main draw method - delegates to helpers."""
self._draw_background(context)
self._draw_timeline_axis(context)
events_with_y_pos = self._calculate_and_draw_events(context)
self._draw_connections(context, events_with_y_pos)
self._draw_year_markers(context)
```
### 3.2 Magic Numbers (MEDIUM PRIORITY)
**Location:** Throughout codebase
**Problem:** Hard-coded values make code harder to maintain:
- `600` (clickable width)
- `30` (min spacing)
- `16` (padding)
- `25` (label offset)
- `10` (marker area padding)
**Recommendation:** Move to constants:
```python
CLICKABLE_AREA_WIDTH = 600
MIN_LABEL_SPACING = 30
LABEL_PADDING = 16
LABEL_X_OFFSET = 25
MARKER_CLICK_PADDING = 10
```
### 3.3 Tuple-Based Data Structure (LOW PRIORITY)
**Location:** `self.events` (line 231)
**Problem:** Using tuples with index access (`prev_data[6]`) is error-prone and hard to read.
**Recommendation:** Use dataclass or named tuple:
```python
from dataclasses import dataclass
@dataclass
class TimelineEvent:
date_sort: int
date_obj: Date
event: Event
person: Person | None
event_type: EventType
expanded: bool
y_pos: float
```
---
## 4. Error Handling
### 4.1 Bare Exception Handlers (MEDIUM PRIORITY)
**Location:** 26 instances of bare `except:` clauses
**Problem:** Catching all exceptions without logging makes debugging difficult.
**Recommendation:**
```python
# Instead of:
except:
pass
# Use:
except Exception as e:
# Log error for debugging
import logging
logging.warning(f"Error processing event: {e}", exc_info=True)
pass
```
### 4.2 Database Access Errors (LOW PRIORITY)
**Location:** `collect_events()`, `person_updated()`, etc.
**Problem:** Database errors are silently ignored.
**Recommendation:** Add specific exception handling for database errors.
---
## 5. Code Quality Improvements
### 5.1 Type Hints (LOW PRIORITY)
**Recommendation:** Add type hints for better IDE support and documentation:
```python
def collect_events(self) -> None:
def find_event_at_position(self, x: float, y: float) -> int | None:
def draw_event_marker(self, context: cairo.Context, x: float, y: float,
event_type: EventType, is_hovered: bool = False,
is_selected: bool = False) -> None:
```
### 5.2 Docstrings (LOW PRIORITY)
**Status:** Most methods have docstrings, but some could be more detailed.
**Recommendation:** Add parameter and return value documentation.
### 5.3 Constants Organization (LOW PRIORITY)
**Location:** Lines 63-72
**Recommendation:** Group related constants:
```python
# Timeline Layout
TIMELINE_MARGIN_LEFT = 150
TIMELINE_MARGIN_RIGHT = 50
TIMELINE_MARGIN_TOP = 50
TIMELINE_MARGIN_BOTTOM = 50
TIMELINE_LINE_WIDTH = 3
# Event Display
EVENT_MARKER_SIZE = 10
EVENT_SPACING = 80
LABEL_X_OFFSET = 25
MIN_LABEL_SPACING = 30
```
---
## 6. User Experience Enhancements
### 6.1 Keyboard Navigation (NEW FEATURE)
**Recommendation:** Add keyboard shortcuts:
- Arrow keys to navigate between events
- Enter to select person
- Space to expand/collapse event
### 6.2 Event Filtering (NEW FEATURE)
**Recommendation:** Add filter options:
- Filter by event type
- Filter by person
- Date range filtering
### 6.3 Export Functionality (NEW FEATURE)
**Recommendation:** Add export options:
- Export timeline as image (PNG/SVG)
- Export event list as CSV
### 6.4 Configuration Options (NEW FEATURE)
**Recommendation:** Add user preferences:
- Customizable colors for event types
- Adjustable spacing and margins
- Font size preferences
---
## 7. Testing & Reliability
### 7.1 Unit Tests (NEW)
**Recommendation:** Add unit tests for:
- Event collection logic
- Collision detection algorithm
- Date calculations
- Coordinate transformations
### 7.2 Edge Case Handling (MEDIUM PRIORITY)
**Issues to address:**
- Very large families (100+ events)
- Events with no dates
- Events spanning very long time periods
- Zoom at extreme levels
---
## 8. Memory Optimization
### 8.1 Event Data Caching (LOW PRIORITY)
**Recommendation:** Cache formatted strings and calculated values to avoid repeated computation.
### 8.2 Tooltip Window Management (LOW PRIORITY)
**Location:** `show_tooltip()` (line 780)
**Recommendation:** Reuse tooltip window instead of creating new one each time.
---
## Priority Summary
### High Priority (Do First)
1. ✅ Extract collision detection to shared method
2. ✅ Cache adjusted positions to improve mouse interaction performance
3. ✅ Replace bare `except:` with specific exception handling
### Medium Priority (Do Soon)
1. Extract label text generation to shared method
2. Break down long methods into smaller functions
3. Replace magic numbers with named constants
4. Optimize zoom operations (don't re-collect events)
### Low Priority (Nice to Have)
1. Add type hints
2. Use dataclasses for event data structure
3. Add keyboard navigation
4. Add event filtering
5. Add unit tests
---
## Estimated Impact
**Performance Improvements:**
- Mouse interaction: 50-70% faster with caching
- Zoom operations: 30-40% faster by avoiding event re-collection
**Code Quality:**
- Reduced duplication: ~100 lines of code
- Improved maintainability: Easier to modify and extend
- Better error handling: Easier debugging
**User Experience:**
- Smoother interactions
- More features (keyboard nav, filtering, export)
- Better customization options
---
## Implementation Order
1. **Phase 1 (Quick Wins):**
- Extract collision detection method
- Add constants for magic numbers
- Cache adjusted positions
2. **Phase 2 (Refactoring):**
- Extract label text generation
- Break down long methods
- Improve error handling
3. **Phase 3 (Enhancements):**
- Add type hints
- Use dataclasses
- Add new features (keyboard nav, filtering)
---
## Conclusion
The MyTimeline plugin is functional and well-designed, but would benefit significantly from:
- Reducing code duplication
- Improving performance through caching
- Better code organization
- Enhanced error handling
These improvements would make the codebase more maintainable, performant, and extensible.

View File

@ -1,394 +0,0 @@
# MyTimeline Plugin - Additional Code Analysis & Improvements
## Executive Summary
After removing expand/collapse functionality and implementing previous enhancements, the codebase is in good shape. However, there are still several opportunities for improvement in code quality, maintainability, and robustness.
**Current State:**
- Total Lines: 1,269
- Methods: 28
- Average Method Length: 35.6 lines
- Some methods exceed 100 lines (complexity concern)
---
## 1. Tuple Indexing Issues (MEDIUM PRIORITY)
### 1.1 Numeric Index Access
**Location:** Throughout codebase, especially `_calculate_adjusted_positions()` (line 573)
**Problem:** Using numeric indices like `prev_data[5]` is error-prone and hard to maintain.
**Current Code:**
```python
prev_y_pos = prev_data[5] # Get y_pos from previous event (index 5 now)
```
**Impact:**
- Easy to introduce bugs when tuple structure changes
- Code is hard to read and understand
- No type safety
**Recommendation:** Use named tuples or dataclasses:
```python
from dataclasses import dataclass
@dataclass
class TimelineEvent:
date_sort: int
date_obj: Date
event: Event
person: Person | None
event_type: EventType
y_pos: float
# Then use:
prev_y_pos = prev_data.y_pos # Much clearer!
```
**Benefits:**
- Type safety
- Better IDE support
- Self-documenting code
- Easier refactoring
---
## 2. Long Methods (MEDIUM PRIORITY)
### 2.1 `collect_person_events` - 130 lines
**Location:** Lines 457-586
**Problem:** This nested function is very long and handles multiple responsibilities.
**Recommendation:** Break into smaller functions:
```python
def _collect_person_event_refs(self, person):
"""Collect event references from a person."""
# Extract event reference collection logic
def _process_event_ref(self, event_ref, person_obj):
"""Process a single event reference."""
# Extract event processing logic
```
### 2.2 `show_tooltip` - 100 lines
**Location:** Lines 804-903
**Problem:** Handles both person events and single events with complex formatting.
**Recommendation:** Split into helper methods:
```python
def _format_person_tooltip(self, person, person_events):
"""Format tooltip for person with multiple events."""
def _format_single_event_tooltip(self, event, event_type):
"""Format tooltip for single event."""
```
### 2.3 `draw_event_marker` - 94 lines
**Location:** Lines 994-1087
**Problem:** Handles multiple shape types and styling logic.
**Recommendation:** Extract shape-specific logic:
```python
def _draw_marker_shadow(self, context, x, y, size):
"""Draw shadow for marker."""
def _draw_marker_gradient(self, context, x, y, size, color):
"""Draw gradient fill for marker."""
```
### 2.4 `on_draw` - 90 lines
**Location:** Lines 904-993
**Problem:** Handles multiple drawing responsibilities.
**Recommendation:** Break into helper methods:
```python
def _draw_background(self, context, width, height):
"""Draw background gradient."""
def _draw_timeline_axis(self, context, timeline_x, y_start, y_end):
"""Draw timeline axis with shadow."""
def _draw_events(self, context, events_with_y_pos, timeline_x):
"""Draw all events."""
```
---
## 3. Cache Management (LOW PRIORITY)
### 3.1 Cache Invalidation Logic
**Location:** `_get_adjusted_events()` (line 726)
**Problem:** Cache invalidation checks context object identity, which may not be reliable.
**Current Code:**
```python
if (self._adjusted_events_cache is not None and
self._cache_context == context and
self._cache_timeline_y_start == timeline_y_start and
self._cache_timeline_y_end == timeline_y_end):
```
**Recommendation:** Use a hash-based cache key:
```python
def _get_cache_key(self, timeline_y_start, timeline_y_end):
"""Generate cache key for adjusted events."""
return hash((len(self.events), timeline_y_start, timeline_y_end))
```
**Benefits:**
- More reliable cache invalidation
- Don't need to store context object
- Better performance
---
## 4. Code Organization (LOW PRIORITY)
### 4.1 Method Grouping
**Recommendation:** Group related methods together:
- Drawing methods: `on_draw`, `draw_event_marker`, `draw_event_label`, `draw_year_markers`, `draw_person_connections`
- Event handling: `collect_events`, `collect_person_events`, `_get_adjusted_events`
- Interaction: `on_button_press`, `on_motion_notify`, `on_leave_notify`, `find_event_at_position`
- UI: `build_widget`, `build_tree`, `goto_handle`
### 4.2 Constants Organization
**Status:** Already well-organized, but could add more:
- Add constants for colors used in drawing
- Add constants for font sizes
- Add constants for padding values
---
## 5. Error Handling Improvements (LOW PRIORITY)
### 5.1 More Specific Exceptions
**Current:** Most handlers use generic `Exception`
**Recommendation:** Use more specific exceptions where possible:
```python
except (AttributeError, KeyError) as e:
# Handle specific database access errors
except ValueError as e:
# Handle date parsing errors
```
### 5.2 Logging
**Recommendation:** Add logging for debugging:
```python
import logging
logger = logging.getLogger(__name__)
try:
# code
except Exception as e:
logger.warning(f"Error processing event: {e}", exc_info=True)
```
---
## 6. Performance Optimizations (LOW PRIORITY)
### 6.1 Repeated Calculations
**Location:** `on_draw()`, `find_event_at_position()`
**Problem:** Date range calculation is repeated.
**Recommendation:** Cache date range:
```python
@property
def date_range(self):
"""Calculate and cache date range."""
if not hasattr(self, '_cached_date_range') or self._cached_date_range is None:
if self.events:
min_date = min(event[0] for event in self.events)
max_date = max(event[0] for event in self.events)
self._cached_date_range = max_date - min_date if max_date != min_date else 1
else:
self._cached_date_range = 1
return self._cached_date_range
```
### 6.2 Tooltip Window Reuse
**Location:** `show_tooltip()` (line 804)
**Problem:** Creates new tooltip window each time.
**Recommendation:** Reuse tooltip window:
```python
if not hasattr(self, '_tooltip_window') or self._tooltip_window is None:
self._tooltip_window = Gtk.Window(type=Gtk.WindowType.POPUP)
# ... setup
# Reuse existing window
```
---
## 7. Type Safety (LOW PRIORITY)
### 7.1 Type Hints
**Recommendation:** Add type hints for better IDE support:
```python
from typing import Optional, List, Tuple
def collect_events(self) -> None:
def find_event_at_position(self, x: float, y: float) -> Optional[int]:
def draw_event_marker(self, context: cairo.Context, x: float, y: float,
event_type: EventType, is_hovered: bool = False,
is_selected: bool = False) -> None:
```
**Benefits:**
- Better IDE autocomplete
- Catch type errors early
- Self-documenting code
---
## 8. Code Duplication (LOW PRIORITY)
### 8.1 Date Range Calculation
**Location:** `on_draw()`, `_get_adjusted_events()`, `find_event_at_position()`
**Problem:** Date range calculation is duplicated.
**Recommendation:** Extract to method:
```python
def _calculate_date_range(self):
"""Calculate date range from events."""
if not self.events:
return 1
min_date = min(event[0] for event in self.events)
max_date = max(event[0] for event in self.events)
return max_date - min_date if max_date != min_date else 1
```
### 8.2 Y Position Calculation
**Location:** Multiple places
**Problem:** Y position calculation formula is repeated.
**Recommendation:** Extract to method:
```python
def _calculate_y_position(self, date_sort, min_date, date_range,
timeline_y_start, timeline_y_end):
"""Calculate Y position for an event based on date."""
return timeline_y_start + (
(date_sort - min_date) / date_range
) * (timeline_y_end - timeline_y_start)
```
---
## 9. Magic Numbers (LOW PRIORITY)
### 9.1 Remaining Magic Numbers
**Locations:**
- Line 567: `text_height + LABEL_PADDING` (already using constant, good!)
- Line 800: `500` (tooltip width?)
- Line 1125: `padding = 8` (could be constant)
- Line 1133: `radius = 5` (could be constant)
**Recommendation:** Add constants:
```python
TOOLTIP_MAX_WIDTH = 500
LABEL_BACKGROUND_PADDING = 8
LABEL_BACKGROUND_RADIUS = 5
```
---
## 10. Documentation (LOW PRIORITY)
### 10.1 Docstring Improvements
**Recommendation:** Add parameter and return value documentation:
```python
def draw_event_marker(self, context, x, y, event_type, is_hovered=False, is_selected=False):
"""
Draw a marker for an event with modern styling.
Args:
context: Cairo drawing context
x: X coordinate for marker center
y: Y coordinate for marker center
event_type: Type of event (determines color and shape)
is_hovered: Whether marker is currently hovered
is_selected: Whether marker belongs to selected person
Returns:
None
"""
```
---
## Priority Summary
### Medium Priority (Consider Soon)
1. ✅ Replace tuple indexing with named tuples/dataclasses
2. ✅ Break down long methods (collect_person_events, show_tooltip, draw_event_marker, on_draw)
3. ✅ Extract repeated calculations (date range, Y position)
### Low Priority (Nice to Have)
1. Improve cache management
2. Add type hints
3. Add more specific exception handling
4. Add logging
5. Extract remaining magic numbers
6. Improve docstrings
7. Reuse tooltip window
---
## Estimated Impact
**Code Quality:**
- Better maintainability with named tuples
- Easier to understand with shorter methods
- Better type safety with type hints
**Performance:**
- Slight improvement from caching date range
- Better cache management
**Developer Experience:**
- Better IDE support
- Easier debugging with logging
- Self-documenting code
---
## Implementation Order
1. **Phase 1 (Quick Wins):**
- Extract date range calculation
- Extract Y position calculation
- Add remaining constants
2. **Phase 2 (Refactoring):**
- Break down long methods
- Replace tuple indexing with named tuples
3. **Phase 3 (Enhancements):**
- Add type hints
- Improve error handling
- Add logging
- Improve docstrings
---
## Conclusion
The codebase is in good shape after recent improvements. The main areas for further enhancement are:
- Using named tuples/dataclasses instead of numeric indexing
- Breaking down long methods for better maintainability
- Extracting repeated calculations
- Adding type hints for better IDE support
These improvements would make the code more maintainable, type-safe, and easier to understand.