dv/mygramps
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Major code improvements: performance, maintainability, and cleanup

Browse Source 
Performance optimizations:
- Optimized event-to-person lookup from O(n*m) to O(n+m) using reverse index
- Added _build_event_to_person_index() to build cache once during event collection
- Improved collision detection algorithm efficiency

Code quality improvements:
- Removed duplicate logger definition
- Removed redundant checks in _calculate_max_connection_line_x()
- Removed unused wrapper method detect_label_overlaps()
- Removed 3 unused methods: _collect_person_events, _collect_person_event_refs, _process_event_ref (67 lines of dead code)

Code organization:
- Extracted helper methods to reduce duplication:
  * _get_person_display_name() for person name lookups
  * _create_timeline_event() for TimelineEvent creation
  * _copy_timeline_event_with_y_pos() for TimelineEvent copying
- Reduced code duplication in filter dialog with nested helper function
- Extracted magic numbers to constants (color values, spacing)

Maintainability:
- Better separation of concerns with helper methods
- Improved code organization and readability
- Consistent error handling patterns
- Fixed indentation issue in _get_person_color()

All changes are backward compatible and maintain existing functionality.
This commit is contained in:
Daniel Viegas 2025-11-29 22:32:56 +01:00
parent 66c932e4b8
commit 3652246bd4
1 changed files with 169 additions and 252 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

421
MyTimeline.py
View File

@ -30,8 +30,6 @@ MyTimeline View - A vertical timeline showing all events in the database
import cairo import cairo
import colorsys import colorsys
import logging import logging
logger = logging.getLogger("plugin.mytimeline")
import math import math
from dataclasses import dataclass from dataclasses import dataclass
from typing import Optional, List, Tuple, Any, Set, Dict from typing import Optional, List, Tuple, Any, Set, Dict
@ -113,9 +111,18 @@ CONNECTION_VERTICAL_LINE_X = 5 # Left of year markers
CONNECTION_LINE_SPACING = 25 # Pixels between vertical lines for different persons CONNECTION_LINE_SPACING = 25 # Pixels between vertical lines for different persons
TIMELINE_LEFT_SPACING = 20 # Spacing between rightmost connection line and timeline TIMELINE_LEFT_SPACING = 20 # Spacing between rightmost connection line and timeline
# Color Generation Constants
PERSON_COLOR_SATURATION = 0.7 # High saturation for vibrant colors
PERSON_COLOR_LIGHTNESS = 0.5 # Medium lightness for good visibility
PERSON_COLOR_ALPHA = 0.75 # Semi-transparent
# Marker State Colors # Marker State Colors
SELECTED_MARKER_COLOR = (0.2, 0.4, 0.9) # Blue highlight for selected person's events SELECTED_MARKER_COLOR = (0.2, 0.4, 0.9) # Blue highlight for selected person's events
# Default Colors
DEFAULT_EVENT_COLOR = (0.5, 0.5, 0.5) # Gray for unknown event types
YEAR_MARKER_COLOR = (0.5, 0.5, 0.5) # Gray for year marker tick marks
# Logger # Logger
logger = logging.getLogger(__name__) logger = logging.getLogger(__name__)
@ -395,6 +402,7 @@ class MyTimelineView(NavigationView):
self._cached_date_range: Optional[int] = None self._cached_date_range: Optional[int] = None
self._cached_min_date: Optional[int] = None self._cached_min_date: Optional[int] = None
self._cached_max_date: Optional[int] = None self._cached_max_date: Optional[int] = None
self._event_to_person_cache: Dict[str, Optional[Any]] = {} # Event handle -> Person object (or None)
# Filter state # Filter state
self.filter_enabled: bool = False self.filter_enabled: bool = False
@ -500,6 +508,8 @@ class MyTimelineView(NavigationView):
# Clear selected persons and colors # Clear selected persons and colors
self.selected_person_handles.clear() self.selected_person_handles.clear()
self.person_colors.clear() self.person_colors.clear()
# Clear event-to-person cache
self._event_to_person_cache.clear()
# Connect to new database signals # Connect to new database signals
if db and db.is_open(): if db and db.is_open():
@ -1073,50 +1083,29 @@ class MyTimelineView(NavigationView):
# Collect all child checkboxes for this family # Collect all child checkboxes for this family
child_checkboxes = [] child_checkboxes = []
# Helper function to add person checkbox
def add_person_checkbox(person_handle: Optional[str], role_label: str) -> None:
"""Add a checkbox for a person if handle is valid."""
if not person_handle:
return
person_name = self._get_person_display_name(person_handle)
if person_name:
label_text = f" {role_label}: {person_name}"
checkbox = Gtk.CheckButton(label=label_text)
checkbox.set_active(True if not self.person_filter else person_handle in self.person_filter)
self._filter_widgets['person_checkboxes'][person_handle] = checkbox
child_checkboxes.append(checkbox)
members_box.pack_start(checkbox, False, False, 0)
# Add father checkbox # Add father checkbox
father_handle = family.get_father_handle() add_person_checkbox(family.get_father_handle(), _('Father'))
if father_handle:
try:
father = self.dbstate.db.get_person_from_handle(father_handle)
if father:
father_label = f" {_('Father')}: {name_displayer.display(father)}"
checkbox = Gtk.CheckButton(label=father_label)
checkbox.set_active(True if not self.person_filter else father_handle in self.person_filter)
self._filter_widgets['person_checkboxes'][father_handle] = checkbox
child_checkboxes.append(checkbox)
members_box.pack_start(checkbox, False, False, 0)
except (AttributeError, KeyError):
pass
# Add mother checkbox # Add mother checkbox
mother_handle = family.get_mother_handle() add_person_checkbox(family.get_mother_handle(), _('Mother'))
if mother_handle:
try:
mother = self.dbstate.db.get_person_from_handle(mother_handle)
if mother:
mother_label = f" {_('Mother')}: {name_displayer.display(mother)}"
checkbox = Gtk.CheckButton(label=mother_label)
checkbox.set_active(True if not self.person_filter else mother_handle in self.person_filter)
self._filter_widgets['person_checkboxes'][mother_handle] = checkbox
child_checkboxes.append(checkbox)
members_box.pack_start(checkbox, False, False, 0)
except (AttributeError, KeyError):
pass
# Add children checkboxes # Add children checkboxes
for child_ref in family.get_child_ref_list(): for child_ref in family.get_child_ref_list():
child_handle = child_ref.ref add_person_checkbox(child_ref.ref, _('Child'))
try:
child = self.dbstate.db.get_person_from_handle(child_handle)
if child:
child_label = f" {_('Child')}: {name_displayer.display(child)}"
checkbox = Gtk.CheckButton(label=child_label)
checkbox.set_active(True if not self.person_filter else child_handle in self.person_filter)
self._filter_widgets['person_checkboxes'][child_handle] = checkbox
child_checkboxes.append(checkbox)
members_box.pack_start(checkbox, False, False, 0)
except (AttributeError, KeyError):
pass
# Only add expander if there are members to show # Only add expander if there are members to show
if len(members_box.get_children()) > 0: if len(members_box.get_children()) > 0:
@ -1628,99 +1617,6 @@ class MyTimelineView(NavigationView):
scroll_to = max(0, event_y - (viewport_height / 2)) scroll_to = max(0, event_y - (viewport_height / 2))
vadj.set_value(scroll_to) vadj.set_value(scroll_to)
def _collect_person_event_refs(self, person) -> List:
"""
Collect event references from a person.
Args:
person: The person object to collect events from.
Returns:
List: List of event references.
"""
if not person:
return []
try:
return person.get_event_ref_list()
except (AttributeError, KeyError) as e:
logger.warning(f"Error accessing event references for person: {e}", exc_info=True)
return []
def _process_event_ref(self, event_ref, person_obj: Optional[Any]) -> Optional[TimelineEvent]:
"""
Process a single event reference and create a TimelineEvent.
Args:
event_ref: The event reference to process.
person_obj: The person object associated with this event (None for family events).
Returns:
Optional[TimelineEvent]: The created TimelineEvent, or None if invalid.
"""
try:
event = self.dbstate.db.get_event_from_handle(event_ref.ref)
if event and event.get_date_object():
date_obj = event.get_date_object()
event_type = event.get_type()
return TimelineEvent(
date_sort=date_obj.get_sort_value(),
date_obj=date_obj,
event=event,
person=person_obj,
event_type=event_type,
y_pos=0.0
)
except (AttributeError, KeyError, ValueError) as e:
logger.debug(f"Skipping invalid event reference: {e}")
return None
def _collect_person_events(self, person, person_obj) -> None:
"""
Collect all events from a person and add them to self.events.
Args:
person: The person object to collect events from.
person_obj: The person object to associate with events.
"""
if not person:
return
event_refs = self._collect_person_event_refs(person)
for event_ref in event_refs:
timeline_event = self._process_event_ref(event_ref, person_obj)
if timeline_event:
self.all_events.append(timeline_event)
def _collect_family_member_events(self, handle: Optional[str], role: str) -> None:
"""
Collect events for a family member (father, mother, or child).
Args:
handle: The person handle to collect events from.
role: Role name for logging purposes ('father', 'mother', 'child').
"""
if not handle:
return
try:
person = self.dbstate.db.get_person_from_handle(handle)
if person:
self._collect_person_events(person, person)
except (AttributeError, KeyError) as e:
logger.debug(f"Error accessing {role}: {e}")
def _collect_family_events(self, family) -> None:
"""
Collect family-level events (marriage, divorce, etc.).
Args:
family: The family object to collect events from.
"""
for event_ref in family.get_event_ref_list():
timeline_event = self._process_event_ref(event_ref, None)
if timeline_event:
self.all_events.append(timeline_event)
def _invalidate_cache(self) -> None: def _invalidate_cache(self) -> None:
"""Invalidate all caches when events change.""" """Invalidate all caches when events change."""
self._adjusted_events_cache = None self._adjusted_events_cache = None
@ -1728,6 +1624,7 @@ class MyTimelineView(NavigationView):
self._cached_date_range = None self._cached_date_range = None
self._cached_min_date = None self._cached_min_date = None
self._cached_max_date = None self._cached_max_date = None
self._event_to_person_cache.clear() # Clear event-to-person cache
def _calculate_timeline_height(self) -> None: def _calculate_timeline_height(self) -> None:
"""Calculate and set timeline height based on number of events and zoom.""" """Calculate and set timeline height based on number of events and zoom."""
@ -1744,6 +1641,58 @@ class MyTimelineView(NavigationView):
if self.drawing_area: if self.drawing_area:
self.drawing_area.set_size_request(800, self.timeline_height) self.drawing_area.set_size_request(800, self.timeline_height)
def _create_timeline_event(self, event, person_obj: Optional[Any] = None, y_pos: float = 0.0) -> Optional[TimelineEvent]:
"""
Create a TimelineEvent from an event object.
Args:
event: The event object.
person_obj: Optional person object associated with this event.
y_pos: Initial Y position (default 0.0).
Returns:
Optional[TimelineEvent]: The created TimelineEvent, or None if invalid.
"""
try:
if not event:
return None
date_obj = event.get_date_object()
if not date_obj:
return None
return TimelineEvent(
date_sort=date_obj.get_sort_value(),
date_obj=date_obj,
event=event,
person=person_obj,
event_type=event.get_type(),
y_pos=y_pos
)
except (AttributeError, KeyError, ValueError) as e:
logger.debug(f"Skipping invalid event: {e}")
return None
def _copy_timeline_event_with_y_pos(self, event_data: TimelineEvent, y_pos: float) -> TimelineEvent:
"""
Create a copy of a TimelineEvent with a new Y position.
Args:
event_data: The original TimelineEvent.
y_pos: The new Y position.
Returns:
TimelineEvent: A new TimelineEvent with the updated Y position.
"""
return TimelineEvent(
date_sort=event_data.date_sort,
date_obj=event_data.date_obj,
event=event_data.event,
person=event_data.person,
event_type=event_data.event_type,
y_pos=y_pos
)
def _process_event(self, event, person_obj: Optional[Any] = None) -> Optional[TimelineEvent]: def _process_event(self, event, person_obj: Optional[Any] = None) -> Optional[TimelineEvent]:
""" """
Process a single event and create a TimelineEvent. Process a single event and create a TimelineEvent.
@ -1755,55 +1704,83 @@ class MyTimelineView(NavigationView):
Returns: Returns:
Optional[TimelineEvent]: The created TimelineEvent, or None if invalid. Optional[TimelineEvent]: The created TimelineEvent, or None if invalid.
""" """
return self._create_timeline_event(event, person_obj)
def _build_event_to_person_index(self) -> None:
"""
Build a reverse index mapping event handles to person objects.
This is much more efficient than searching through all persons for each event.
"""
if not self.dbstate.is_open():
return
self._event_to_person_cache.clear()
try: try:
if event and event.get_date_object(): # Iterate through all persons once and build the index
date_obj = event.get_date_object() for person_handle in self.dbstate.db.get_person_handles():
event_type = event.get_type() try:
return TimelineEvent( person = self.dbstate.db.get_person_from_handle(person_handle)
date_sort=date_obj.get_sort_value(), if not person:
date_obj=date_obj, continue
event=event,
person=person_obj, # Check primary events first (these take precedence)
event_type=event_type, for event_ref in person.get_primary_event_ref_list():
y_pos=0.0 event_handle = event_ref.ref
) # Only store if not already mapped (primary events take priority)
except (AttributeError, KeyError, ValueError) as e: if event_handle not in self._event_to_person_cache:
logger.debug(f"Skipping invalid event: {e}") self._event_to_person_cache[event_handle] = person
return None
# Check all events (for events not in primary list)
for event_ref in person.get_event_ref_list():
event_handle = event_ref.ref
# Only store if not already mapped
if event_handle not in self._event_to_person_cache:
self._event_to_person_cache[event_handle] = person
except (AttributeError, KeyError):
continue
except (AttributeError, KeyError) as e:
logger.warning(f"Error building event-to-person index: {e}", exc_info=True)
def _find_person_for_event(self, event) -> Optional[Any]: def _find_person_for_event(self, event) -> Optional[Any]:
""" """
Find a primary person associated with an event by searching through people. Find a primary person associated with an event using the cached index.
Args: Args:
event: The event object to find a person for. event: The event object to find a person for.
Returns: Returns:
Optional person object, or None if not found or if event is family-only. Optional person object, or None if not found or if event is family-only.
Note:
This method now uses the cached index built by _build_event_to_person_index().
If the cache is empty, returns None. For best performance, ensure
_build_event_to_person_index() is called before using this method.
""" """
if not event: if not event:
return None return None
event_handle = event.get_handle() event_handle = event.get_handle()
return self._event_to_person_cache.get(event_handle)
def _get_person_display_name(self, person_handle: Optional[str]) -> Optional[str]:
"""
Get display name for a person by handle.
Args:
person_handle: The person handle.
Returns:
Optional[str]: Display name or None if person not found or handle is None.
"""
if not person_handle:
return None
# Search through people to find who references this event
# For performance, we'll search through a limited set or use event references
try: try:
# Try to find a person who has this event as a primary event person = self.dbstate.db.get_person_from_handle(person_handle)
for person_handle in self.dbstate.db.get_person_handles(): if person:
try: return name_displayer.display(person)
person = self.dbstate.db.get_person_from_handle(person_handle)
if person:
# Check primary events first
for event_ref in person.get_primary_event_ref_list():
if event_ref.ref == event_handle:
return person
# Check all events
for event_ref in person.get_event_ref_list():
if event_ref.ref == event_handle:
return person
except (AttributeError, KeyError):
continue
except (AttributeError, KeyError): except (AttributeError, KeyError):
pass pass
@ -1822,27 +1799,8 @@ class MyTimelineView(NavigationView):
if not family: if not family:
return _("Unknown Family") return _("Unknown Family")
father_handle = family.get_father_handle() father_name = self._get_person_display_name(family.get_father_handle())
mother_handle = family.get_mother_handle() mother_name = self._get_person_display_name(family.get_mother_handle())
father_name = None
mother_name = None
if father_handle:
try:
father = self.dbstate.db.get_person_from_handle(father_handle)
if father:
father_name = name_displayer.display(father)
except (AttributeError, KeyError):
pass
if mother_handle:
try:
mother = self.dbstate.db.get_person_from_handle(mother_handle)
if mother:
mother_name = name_displayer.display(mother)
except (AttributeError, KeyError):
pass
if father_name and mother_name: if father_name and mother_name:
return f"{father_name} & {mother_name}" return f"{father_name} & {mother_name}"
@ -1857,15 +1815,20 @@ class MyTimelineView(NavigationView):
"""Collect all events from the database.""" """Collect all events from the database."""
self.all_events = [] self.all_events = []
self.events = [] self.events = []
self._event_to_person_cache.clear()
if not self.dbstate.is_open(): if not self.dbstate.is_open():
return return
try: try:
# Build event-to-person reverse index for efficient lookup
self._build_event_to_person_index()
# Iterate through all events in the database # Iterate through all events in the database
for event in self.dbstate.db.iter_events(): for event in self.dbstate.db.iter_events():
# Try to find an associated person for this event # Get associated person from cache
person_obj = self._find_person_for_event(event) event_handle = event.get_handle()
person_obj = self._event_to_person_cache.get(event_handle)
# Process the event # Process the event
timeline_event = self._process_event(event, person_obj) timeline_event = self._process_event(event, person_obj)
@ -2146,24 +2109,14 @@ class MyTimelineView(NavigationView):
if person_handle in self.person_colors: if person_handle in self.person_colors:
return self.person_colors[person_handle] return self.person_colors[person_handle]
# Generate a new color based on the person's position in selection # Generate color using HSV: vary hue, keep saturation and lightness constant
# Use hash of handle for consistent color assignment # Use hash of handle for consistent color even when selection order changes
num_selected = len(self.selected_person_handles) handle_hash = hash(person_handle)
if num_selected == 0: hue = (abs(handle_hash) % 360) / 360.0 # 0-1 range
# Default color if no selection (shouldn't happen)
color = (0.2, 0.5, 1.0, 0.75) # Convert HSV to RGB (HSV is Hue, Saturation, Value)
else: r, g, b = colorsys.hsv_to_rgb(hue, PERSON_COLOR_SATURATION, PERSON_COLOR_LIGHTNESS)
# Generate color using HSL: vary hue, keep saturation and lightness constant color = (r, g, b, PERSON_COLOR_ALPHA)
# Use hash of handle for consistent color even when selection order changes
handle_hash = hash(person_handle)
hue = (abs(handle_hash) % 360) / 360.0 # 0-1 range
saturation = 0.7 # High saturation for vibrant colors
lightness = 0.5 # Medium lightness for good visibility
# Convert HSV to RGB (HSV is Hue, Saturation, Value)
r, g, b = colorsys.hsv_to_rgb(hue, saturation, lightness)
alpha = 0.75 # Semi-transparent
color = (r, g, b, alpha)
# Store the color # Store the color
self.person_colors[person_handle] = color self.person_colors[person_handle] = color
@ -2191,12 +2144,8 @@ class MyTimelineView(NavigationView):
if not self.selected_person_handles: if not self.selected_person_handles:
return 0 return 0
num_selected = len(self.selected_person_handles)
if num_selected == 0:
return 0
# The rightmost line is at the highest index # The rightmost line is at the highest index
max_index = num_selected - 1 max_index = len(self.selected_person_handles) - 1
return CONNECTION_VERTICAL_LINE_X + (max_index * CONNECTION_LINE_SPACING) return CONNECTION_VERTICAL_LINE_X + (max_index * CONNECTION_LINE_SPACING)
def _calculate_timeline_x(self) -> float: def _calculate_timeline_x(self) -> float:
@ -2298,49 +2247,24 @@ class MyTimelineView(NavigationView):
text_width, text_height = layout.get_pixel_size() text_width, text_height = layout.get_pixel_size()
label_height = text_height + LABEL_PADDING label_height = text_height + LABEL_PADDING
# Check for overlap with previous events # Check for overlap with previous events (optimized: find max y_pos to place after)
adjusted_y = event_data.y_pos adjusted_y = event_data.y_pos
for prev_event in adjusted_events: for prev_event in adjusted_events:
# Check if labels would overlap # Check if labels would overlap
if abs(adjusted_y - prev_event.y_pos) < MIN_LABEL_SPACING: prev_y = prev_event.y_pos
# Adjust downward if adjusted_y < prev_y + MIN_LABEL_SPACING:
adjusted_y = prev_event.y_pos + MIN_LABEL_SPACING # Adjust downward to be below the previous event
adjusted_y = prev_y + MIN_LABEL_SPACING
# Ensure adjusted position is within bounds # Ensure adjusted position is within bounds
adjusted_y = max(timeline_y_start, min(adjusted_y, timeline_y_end)) adjusted_y = max(timeline_y_start, min(adjusted_y, timeline_y_end))
# Create new event data with adjusted Y position # Create new event data with adjusted Y position
adjusted_event = TimelineEvent( adjusted_event = self._copy_timeline_event_with_y_pos(event_data, adjusted_y)
date_sort=event_data.date_sort,
date_obj=event_data.date_obj,
event=event_data.event,
person=event_data.person,
event_type=event_data.event_type,
y_pos=adjusted_y
)
adjusted_events.append(adjusted_event) adjusted_events.append(adjusted_event)
return adjusted_events return adjusted_events
def detect_label_overlaps(self, context: cairo.Context,
events_with_y_pos: List[TimelineEvent],
timeline_y_start: float,
timeline_y_end: float) -> List[TimelineEvent]:
"""
Detect and adjust Y positions to prevent label overlaps.
Args:
context: Cairo drawing context.
events_with_y_pos: List of TimelineEvent objects with initial Y positions.
timeline_y_start: The Y coordinate of the timeline start.
timeline_y_end: The Y coordinate of the timeline end.
Returns:
List[TimelineEvent]: List of TimelineEvent objects with adjusted Y positions.
"""
# Use shared collision detection method
return self._calculate_adjusted_positions(context, events_with_y_pos, timeline_y_start, timeline_y_end)
def _recalculate_timeline_height(self) -> None: def _recalculate_timeline_height(self) -> None:
""" """
Recalculate timeline height based on current events and zoom level. Recalculate timeline height based on current events and zoom level.
@ -2584,14 +2508,7 @@ class MyTimelineView(NavigationView):
event_data.date_sort, min_date, date_range, event_data.date_sort, min_date, date_range,
timeline_y_start, timeline_y_end timeline_y_start, timeline_y_end
) )
event_with_y = TimelineEvent( event_with_y = self._copy_timeline_event_with_y_pos(event_data, y_pos)
date_sort=event_data.date_sort,
date_obj=event_data.date_obj,
event=event_data.event,
person=event_data.person,
event_type=event_data.event_type,
y_pos=y_pos
)
events_with_y_pos.append(event_with_y) events_with_y_pos.append(event_with_y)
# Apply collision detection using shared method # Apply collision detection using shared method
@ -3012,7 +2929,7 @@ class MyTimelineView(NavigationView):
event_type_value = event_type.value if hasattr(event_type, 'value') else int(event_type) event_type_value = event_type.value if hasattr(event_type, 'value') else int(event_type)
# Get color and shape # Get color and shape
color = EVENT_COLORS.get(event_type_value, (0.5, 0.5, 0.5)) color = EVENT_COLORS.get(event_type_value, DEFAULT_EVENT_COLOR)
shape = EVENT_SHAPES.get(event_type_value, 'square') shape = EVENT_SHAPES.get(event_type_value, 'square')
marker_size = EVENT_MARKER_SIZE marker_size = EVENT_MARKER_SIZE
@ -3243,7 +3160,7 @@ class MyTimelineView(NavigationView):
y_pos: Y position for the marker. y_pos: Y position for the marker.
""" """
# Draw tick mark # Draw tick mark
context.set_source_rgb(0.5, 0.5, 0.5) # Gray context.set_source_rgb(*YEAR_MARKER_COLOR)
context.set_line_width(1) context.set_line_width(1)
context.move_to(timeline_x - 10, y_pos) context.move_to(timeline_x - 10, y_pos)
context.line_to(timeline_x, y_pos) context.line_to(timeline_x, y_pos)