Additional code quality improvements

- Move datetime imports to module level (follows Python best practices)
  Removed 3 duplicate imports from inside methods

- Extract remaining hardcoded UI constants
  Added FILTER_PAGE_VERTICAL_SPACING, FILTER_CATEGORY_SPACING, FILTER_INDENT_MARGIN
  Added TOOLBAR_SPACING, TOOLBAR_ITEM_MARGIN
  Added TOOLTIP_SEPARATOR_LENGTH
  Replaced all hardcoded spacing/margin values in filter page builders

- Extract common zoom logic into _update_zoom() helper
  Reduces duplication across on_zoom_in, on_zoom_out, on_zoom_reset methods

- Consolidate date handler methods
  Created _sync_year_spin_from_calendar() and _sync_calendar_from_year_spin() helpers
  Created _create_date_from_calendar() helper for date creation
  Reduced ~60 lines of duplicated code in date handlers

- Improve cache management
  Clear event type normalization cache in _invalidate_cache() to prevent stale data

- Improve error messages with more context
  All error messages now specify which operation/component failed
  Makes debugging significantly easier
This commit is contained in:
Daniel Viegas 2025-11-30 00:34:56 +01:00
parent c76735f2b8
commit 6dc5f16a94

View File

@ -29,6 +29,7 @@ MyTimeline View - A vertical timeline showing all events in the database
# -------------------------------------------------------------------------
import cairo
import colorsys
import datetime
import logging
import math
from dataclasses import dataclass
@ -104,10 +105,16 @@ DATE_SORT_MONTH_MULTIPLIER = 100 # Month component in date sort value
# UI Layout Constants
FILTER_PAGE_MARGIN = 10 # Margin for filter page containers
FILTER_PAGE_SPACING = 10 # Spacing in filter page containers
FILTER_PAGE_VERTICAL_SPACING = 5 # Vertical spacing within filter pages
FILTER_CATEGORY_SPACING = 2 # Spacing within category containers
FILTER_INDENT_MARGIN = 20 # Indent margin for nested elements
CALENDAR_CONTAINER_MARGIN = 5 # Margin for calendar containers
CALENDAR_CONTAINER_SPACING = 5 # Spacing in calendar containers
CALENDAR_HORIZONTAL_SPACING = 15 # Spacing between calendar frames
YEAR_SELECTOR_SPACING = 5 # Spacing in year selector boxes
TOOLBAR_SPACING = 5 # Spacing in toolbar
TOOLBAR_ITEM_MARGIN = 10 # Margin for toolbar items
TOOLTIP_SEPARATOR_LENGTH = 30 # Length of tooltip separator line
# Font Constants
FONT_FAMILY = "Sans"
@ -551,7 +558,7 @@ class MyTimelineView(NavigationView):
Gtk.Widget: The main container widget with toolbar and drawing area.
"""
# Main container
main_box = Gtk.Box(orientation=Gtk.Orientation.VERTICAL, spacing=5)
main_box = Gtk.Box(orientation=Gtk.Orientation.VERTICAL, spacing=TOOLBAR_SPACING)
# Toolbar with zoom controls
toolbar = Gtk.Toolbar()
@ -565,8 +572,8 @@ class MyTimelineView(NavigationView):
# Zoom label
self.zoom_label = Gtk.Label(label="100%")
self.zoom_label.set_margin_start(10)
self.zoom_label.set_margin_end(10)
self.zoom_label.set_margin_start(TOOLBAR_ITEM_MARGIN)
self.zoom_label.set_margin_end(TOOLBAR_ITEM_MARGIN)
zoom_item = Gtk.ToolItem()
zoom_item.add(self.zoom_label)
toolbar.insert(zoom_item, 1)
@ -796,14 +803,14 @@ class MyTimelineView(NavigationView):
scrolled = Gtk.ScrolledWindow()
scrolled.set_policy(Gtk.PolicyType.AUTOMATIC, Gtk.PolicyType.AUTOMATIC)
box = Gtk.Box(orientation=Gtk.Orientation.VERTICAL, spacing=5)
box.set_margin_start(10)
box.set_margin_end(10)
box.set_margin_top(10)
box.set_margin_bottom(10)
box = Gtk.Box(orientation=Gtk.Orientation.VERTICAL, spacing=FILTER_PAGE_VERTICAL_SPACING)
box.set_margin_start(FILTER_PAGE_MARGIN)
box.set_margin_end(FILTER_PAGE_MARGIN)
box.set_margin_top(FILTER_PAGE_MARGIN)
box.set_margin_bottom(FILTER_PAGE_MARGIN)
# Select All / Deselect All buttons
button_box = Gtk.Box(orientation=Gtk.Orientation.HORIZONTAL, spacing=5)
button_box = Gtk.Box(orientation=Gtk.Orientation.HORIZONTAL, spacing=FILTER_PAGE_VERTICAL_SPACING)
select_all_btn = Gtk.Button(label=_("Select All"))
select_all_btn.connect("clicked", self._on_select_all_event_types)
deselect_all_btn = Gtk.Button(label=_("Deselect All"))
@ -837,8 +844,8 @@ class MyTimelineView(NavigationView):
box.pack_start(category_checkbox, False, False, 0)
# Create container for event types in this category
category_box = Gtk.Box(orientation=Gtk.Orientation.VERTICAL, spacing=2)
category_box.set_margin_start(20)
category_box = Gtk.Box(orientation=Gtk.Orientation.VERTICAL, spacing=FILTER_CATEGORY_SPACING)
category_box.set_margin_start(FILTER_INDENT_MARGIN)
category_boxes[category] = category_box
box.pack_start(category_box, False, False, 0)
@ -877,11 +884,11 @@ class MyTimelineView(NavigationView):
Returns:
Gtk.Widget: The category filter page.
"""
box = Gtk.Box(orientation=Gtk.Orientation.VERTICAL, spacing=5)
box.set_margin_start(10)
box.set_margin_end(10)
box.set_margin_top(10)
box.set_margin_bottom(10)
box = Gtk.Box(orientation=Gtk.Orientation.VERTICAL, spacing=FILTER_PAGE_VERTICAL_SPACING)
box.set_margin_start(FILTER_PAGE_MARGIN)
box.set_margin_end(FILTER_PAGE_MARGIN)
box.set_margin_top(FILTER_PAGE_MARGIN)
box.set_margin_bottom(FILTER_PAGE_MARGIN)
# Get unique categories
categories = sorted(set(EVENT_CATEGORIES.values()))
@ -906,11 +913,11 @@ class MyTimelineView(NavigationView):
scrolled = Gtk.ScrolledWindow()
scrolled.set_policy(Gtk.PolicyType.AUTOMATIC, Gtk.PolicyType.AUTOMATIC)
box = Gtk.Box(orientation=Gtk.Orientation.VERTICAL, spacing=5)
box.set_margin_start(10)
box.set_margin_end(10)
box.set_margin_top(10)
box.set_margin_bottom(10)
box = Gtk.Box(orientation=Gtk.Orientation.VERTICAL, spacing=FILTER_PAGE_VERTICAL_SPACING)
box.set_margin_start(FILTER_PAGE_MARGIN)
box.set_margin_end(FILTER_PAGE_MARGIN)
box.set_margin_top(FILTER_PAGE_MARGIN)
box.set_margin_bottom(FILTER_PAGE_MARGIN)
# Person checkboxes container - will be populated when dialog is shown
person_checkboxes = {}
@ -1022,7 +1029,6 @@ class MyTimelineView(NavigationView):
calendar_box.set_homogeneous(True)
# Year range for genealogical data
import datetime
current_year = datetime.date.today().year
min_year = MIN_GENEALOGICAL_YEAR
max_year = current_year + 10
@ -1155,10 +1161,10 @@ class MyTimelineView(NavigationView):
family_name = self._get_family_display_name(family)
# Create container for family members
members_box = Gtk.Box(orientation=Gtk.Orientation.VERTICAL, spacing=3)
members_box.set_margin_start(20)
members_box.set_margin_top(5)
members_box.set_margin_bottom(5)
members_box = Gtk.Box(orientation=Gtk.Orientation.VERTICAL, spacing=FILTER_CATEGORY_SPACING + 1)
members_box.set_margin_start(FILTER_INDENT_MARGIN)
members_box.set_margin_top(CALENDAR_CONTAINER_MARGIN)
members_box.set_margin_bottom(CALENDAR_CONTAINER_MARGIN)
# Collect all child checkboxes for this family
child_checkboxes = []
@ -1224,7 +1230,7 @@ class MyTimelineView(NavigationView):
container.show_all()
except (AttributeError, KeyError) as e:
logger.warning(f"Error updating person filter: {e}", exc_info=True)
logger.warning(f"Error updating person filter widgets in filter dialog: {e}", exc_info=True)
def _update_date_range_widgets(self) -> None:
"""
@ -1256,7 +1262,6 @@ class MyTimelineView(NavigationView):
self._update_year_spin_button('date_to_year_spin', to_year, self._on_to_year_changed)
else:
# Reset to current date
import datetime
now = datetime.date.today()
from_calendar.select_month(now.month - 1, now.year)
to_calendar.select_month(now.month - 1, now.year)
@ -1342,20 +1347,22 @@ class MyTimelineView(NavigationView):
# For now, we'll always use calendar dates if they're valid
# The "Clear Dates" button will reset the explicit flag
# Get selected dates from calendars
# Gtk.Calendar.get_date() returns (year, month, day) where month is 0-11
from_year, from_month, from_day = from_calendar.get_date()
to_year, to_month, to_day = to_calendar.get_date()
# Get Date objects from calendar selections
from_date = self._create_date_from_calendar(from_calendar)
to_date = self._create_date_from_calendar(to_calendar)
if from_date is None or to_date is None:
# Show error message for invalid dates
if hasattr(self, 'date_validation_label'):
self.date_validation_label.set_markup(
f"<span color='red'>{_('Error: Invalid date in date range filter')}</span>"
)
logger.warning("Error parsing date range in filter dialog: invalid date from calendar")
self.date_range_filter = None
self.date_range_explicit = False
return
try:
# Create Date objects from calendar selections
# Note: month from calendar is 0-11, Date.set_yr_mon_day expects 1-12
from_date = Date()
from_date.set_yr_mon_day(from_year, from_month + 1, from_day)
min_sort = from_date.get_sort_value()
to_date = Date()
to_date.set_yr_mon_day(to_year, to_month + 1, to_day)
max_sort = to_date.get_sort_value()
# Validate date range
@ -1376,15 +1383,6 @@ class MyTimelineView(NavigationView):
# Set filter - user has selected dates in calendars
self.date_range_filter = (min_sort, max_sort)
self.date_range_explicit = True
except (ValueError, AttributeError, TypeError) as e:
logger.warning(f"Error parsing date range: {e}", exc_info=True)
self.date_range_filter = None
self.date_range_explicit = False
if hasattr(self, 'date_validation_label'):
self.date_validation_label.set_markup(
f"<span color='red'>{_('Error: Invalid date')}</span>"
)
else:
# No calendar widgets, clear filter
self.date_range_filter = None
@ -1435,6 +1433,56 @@ class MyTimelineView(NavigationView):
for checkbox in self._filter_widgets['event_type_checkboxes'].values():
checkbox.set_active(False)
def _sync_year_spin_from_calendar(self, calendar: Gtk.Calendar, spin_key: str, handler: Callable) -> None:
"""
Update year spin button to match calendar date.
Args:
calendar: The calendar widget.
spin_key: Key to find the spin button in _filter_widgets.
handler: Handler function to block/unblock during update.
"""
if spin_key in self._filter_widgets:
year, month, day = calendar.get_date()
year_spin = self._filter_widgets[spin_key]
year_spin.handler_block_by_func(handler)
year_spin.set_value(year)
year_spin.handler_unblock_by_func(handler)
def _sync_calendar_from_year_spin(self, calendar_key: str, new_year: int) -> None:
"""
Update calendar to match year spin button value.
Args:
calendar_key: Key to find the calendar in _filter_widgets.
new_year: The new year value to set.
"""
if calendar_key in self._filter_widgets:
calendar = self._filter_widgets[calendar_key]
current_year, current_month, current_day = calendar.get_date()
# Update calendar to new year, keeping same month and day
calendar.select_month(current_month, new_year)
def _create_date_from_calendar(self, calendar: Gtk.Calendar) -> Optional[Date]:
"""
Create a Date object from calendar selection.
Args:
calendar: The calendar widget.
Returns:
Optional[Date]: Date object if successful, None otherwise.
"""
try:
year, month, day = calendar.get_date()
# Note: month from calendar is 0-11, Date.set_yr_mon_day expects 1-12
date_obj = Date()
date_obj.set_yr_mon_day(year, month + 1, day)
return date_obj
except (ValueError, AttributeError, TypeError) as e:
logger.debug(f"Error creating date from calendar: {e}")
return None
def _on_from_date_selected(self, calendar: Gtk.Calendar) -> None:
"""
Handle From date calendar selection.
@ -1442,14 +1490,7 @@ class MyTimelineView(NavigationView):
Args:
calendar: The calendar widget that was selected.
"""
# Update year spin button to match calendar
if 'date_from_year_spin' in self._filter_widgets:
year, month, day = calendar.get_date()
year_spin = self._filter_widgets['date_from_year_spin']
year_spin.handler_block_by_func(self._on_from_year_changed)
year_spin.set_value(year)
year_spin.handler_unblock_by_func(self._on_from_year_changed)
self._sync_year_spin_from_calendar(calendar, 'date_from_year_spin', self._on_from_year_changed)
self._validate_date_range()
def _on_to_date_selected(self, calendar: Gtk.Calendar) -> None:
@ -1459,14 +1500,7 @@ class MyTimelineView(NavigationView):
Args:
calendar: The calendar widget that was selected.
"""
# Update year spin button to match calendar
if 'date_to_year_spin' in self._filter_widgets:
year, month, day = calendar.get_date()
year_spin = self._filter_widgets['date_to_year_spin']
year_spin.handler_block_by_func(self._on_to_year_changed)
year_spin.set_value(year)
year_spin.handler_unblock_by_func(self._on_to_year_changed)
self._sync_year_spin_from_calendar(calendar, 'date_to_year_spin', self._on_to_year_changed)
self._validate_date_range()
def _on_from_calendar_changed(self, calendar: Gtk.Calendar) -> None:
@ -1476,13 +1510,7 @@ class MyTimelineView(NavigationView):
Args:
calendar: The calendar widget that changed.
"""
# Update year spin button to match calendar
if 'date_from_year_spin' in self._filter_widgets:
year, month, day = calendar.get_date()
year_spin = self._filter_widgets['date_from_year_spin']
year_spin.handler_block_by_func(self._on_from_year_changed)
year_spin.set_value(year)
year_spin.handler_unblock_by_func(self._on_from_year_changed)
self._sync_year_spin_from_calendar(calendar, 'date_from_year_spin', self._on_from_year_changed)
def _on_to_calendar_changed(self, calendar: Gtk.Calendar) -> None:
"""
@ -1491,13 +1519,7 @@ class MyTimelineView(NavigationView):
Args:
calendar: The calendar widget that changed.
"""
# Update year spin button to match calendar
if 'date_to_year_spin' in self._filter_widgets:
year, month, day = calendar.get_date()
year_spin = self._filter_widgets['date_to_year_spin']
year_spin.handler_block_by_func(self._on_to_year_changed)
year_spin.set_value(year)
year_spin.handler_unblock_by_func(self._on_to_year_changed)
self._sync_year_spin_from_calendar(calendar, 'date_to_year_spin', self._on_to_year_changed)
def _on_from_year_changed(self, spin_button: Gtk.SpinButton) -> None:
"""
@ -1506,13 +1528,8 @@ class MyTimelineView(NavigationView):
Args:
spin_button: The year spin button that changed.
"""
if 'date_from_calendar' in self._filter_widgets:
calendar = self._filter_widgets['date_from_calendar']
new_year = int(spin_button.get_value())
current_year, current_month, current_day = calendar.get_date()
# Update calendar to new year, keeping same month and day
calendar.select_month(current_month, new_year)
# Trigger validation
self._sync_calendar_from_year_spin('date_from_calendar', new_year)
self._validate_date_range()
def _on_to_year_changed(self, spin_button: Gtk.SpinButton) -> None:
@ -1522,13 +1539,8 @@ class MyTimelineView(NavigationView):
Args:
spin_button: The year spin button that changed.
"""
if 'date_to_calendar' in self._filter_widgets:
calendar = self._filter_widgets['date_to_calendar']
new_year = int(spin_button.get_value())
current_year, current_month, current_day = calendar.get_date()
# Update calendar to new year, keeping same month and day
calendar.select_month(current_month, new_year)
# Trigger validation
self._sync_calendar_from_year_spin('date_to_calendar', new_year)
self._validate_date_range()
def _on_clear_date_range(self, button: Gtk.Button) -> None:
@ -1544,7 +1556,6 @@ class MyTimelineView(NavigationView):
# Reset to current date (calendars always show a date)
# Get current date and set calendars to it
import datetime
now = datetime.date.today()
from_calendar.select_month(now.month - 1, now.year) # month is 0-11
from_calendar.select_day(now.day)
@ -1682,6 +1693,8 @@ class MyTimelineView(NavigationView):
self._cached_min_date = None
self._cached_max_date = None
self._event_to_person_cache.clear() # Clear event-to-person cache
# Clear event type normalization cache to prevent stale data
self._event_type_normalization_cache.clear()
def _calculate_timeline_height(self) -> None:
"""Calculate and set timeline height based on number of events and zoom."""
@ -1798,7 +1811,7 @@ class MyTimelineView(NavigationView):
except (AttributeError, KeyError):
continue
except (AttributeError, KeyError) as e:
logger.warning(f"Error building event-to-person index: {e}", exc_info=True)
logger.warning(f"Error building event-to-person index from database: {e}", exc_info=True)
def _find_person_for_event(self, event: 'Event') -> Optional['Person']:
"""
@ -1893,7 +1906,7 @@ class MyTimelineView(NavigationView):
self.all_events.append(timeline_event)
except (AttributeError, KeyError) as e:
logger.warning(f"Error collecting events: {e}", exc_info=True)
logger.warning(f"Error collecting events from database: {e}", exc_info=True)
return
# Sort events by date
@ -2395,6 +2408,19 @@ class MyTimelineView(NavigationView):
self._adjusted_events_cache = None
self._cache_key = None
def _update_zoom(self, level: float) -> None:
"""
Update zoom level and refresh the display.
Args:
level: The new zoom level to set.
"""
self.zoom_level = level
self.update_zoom_display()
self._recalculate_timeline_height()
if self.drawing_area:
self.drawing_area.queue_draw()
def on_zoom_in(self, widget: Gtk.Widget) -> None:
"""
Zoom in.
@ -2403,11 +2429,8 @@ class MyTimelineView(NavigationView):
widget: The widget that triggered the zoom (unused).
"""
if self.zoom_level < self.max_zoom:
self.zoom_level = min(self.zoom_level + self.zoom_step, self.max_zoom)
self.update_zoom_display()
self._recalculate_timeline_height() # Only recalculate height, not events
if self.drawing_area:
self.drawing_area.queue_draw()
new_level = min(self.zoom_level + self.zoom_step, self.max_zoom)
self._update_zoom(new_level)
def on_zoom_out(self, widget: Gtk.Widget) -> None:
"""
@ -2417,11 +2440,8 @@ class MyTimelineView(NavigationView):
widget: The widget that triggered the zoom (unused).
"""
if self.zoom_level > self.min_zoom:
self.zoom_level = max(self.zoom_level - self.zoom_step, self.min_zoom)
self.update_zoom_display()
self._recalculate_timeline_height() # Only recalculate height, not events
if self.drawing_area:
self.drawing_area.queue_draw()
new_level = max(self.zoom_level - self.zoom_step, self.min_zoom)
self._update_zoom(new_level)
def on_zoom_reset(self, widget: Gtk.Widget) -> None:
"""
@ -2430,11 +2450,7 @@ class MyTimelineView(NavigationView):
Args:
widget: The widget that triggered the reset (unused).
"""
self.zoom_level = 1.0
self.update_zoom_display()
self._recalculate_timeline_height() # Only recalculate height, not events
if self.drawing_area:
self.drawing_area.queue_draw()
self._update_zoom(1.0)
def on_scroll(self, widget: Gtk.Widget, event: Gdk.Event) -> bool:
"""
@ -2695,7 +2711,7 @@ class MyTimelineView(NavigationView):
if place:
return place.get_title()
except (AttributeError, KeyError) as e:
logger.debug(f"Error accessing place for event: {e}")
logger.debug(f"Error accessing place information for event in tooltip: {e}")
return None
def _format_person_tooltip(self, person: 'Person', person_events: List[TimelineEvent]) -> str:
@ -2711,7 +2727,7 @@ class MyTimelineView(NavigationView):
"""
person_name = name_displayer.display(person)
tooltip_text = f"<b>{person_name}</b>\n"
tooltip_text += "" * 30 + "\n"
tooltip_text += "" * TOOLTIP_SEPARATOR_LENGTH + "\n"
# Sort by date
person_events_sorted = sorted(person_events, key=lambda x: x.date_sort)
@ -3237,7 +3253,7 @@ class MyTimelineView(NavigationView):
if max_year is None or year > max_year:
max_year = year
except (AttributeError, ValueError) as e:
logger.debug(f"Error accessing year from date: {e}")
logger.debug(f"Error extracting year from date for timeline year marker: {e}")
return (min_year, max_year)