Skip to content

Conversation

@tmustier
Copy link
Owner

Summary

Typing pass 4 for chart-engine bar overlays.

This removes broad file-level pyright suppressions from the remaining bar overlay orchestration/segment modules and replaces dynamic access with explicit typed normalization while preserving rendering behavior.

Changes

  • clean_slides/chart_engine/overlay_bar.py

    • removed broad # pyright suppression header
    • added typed coercion helpers for overlay/meta values (list/mapping/numeric/color/font/base-dir parsing)
    • added typed callable wrappers for annotation + txBody template loader imports
    • normalized annotation specs and text-style template loading with typed guards
    • kept existing overlay flow intact (annotations -> segment labels -> totals -> categories -> legend)
  • clean_slides/chart_engine/overlay_bar_segments.py

    • removed broad # pyright suppression header
    • added typed callable wrappers for add_text_label and resolve_txbody_template
    • normalized segment config parsing and geometry extraction with typed helpers
    • retained segment-label placement/offset behavior and preserved rendered-width feedback loop

Validation

  • .venv/bin/ruff check clean_slides/chart_engine/overlay_bar.py clean_slides/chart_engine/overlay_bar_segments.py
  • .venv/bin/pyright clean_slides/chart_engine/overlay_bar.py clean_slides/chart_engine/overlay_bar_segments.py
  • .venv/bin/pyright
  • .venv/bin/pytest -q
  • .venv/bin/pre-commit run --all-files

@tmustier tmustier merged commit 9c252fb into main Feb 12, 2026
7 checks passed
@tmustier tmustier deleted the refactor/chart-engine-overlay-typing-pass-4 branch February 12, 2026 01:55
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 205a404a03

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +198 to +202
def _number_list(value: object) -> list[Number]:
result: list[Number] = []
for item in _list(value):
if isinstance(item, (int, float)):
result.append(item)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve sparse offset indices during numeric normalization

The new _number_list compacts arrays by removing non-numeric entries, but downstream category/total offset lookups are index-based and previously tolerated placeholders via _number(..., 0). With configs like category_label_offsets: [null, 12000] or total_label_offsets_x: [null, -8000], the second offset now shifts onto the first category and later categories lose their intended overrides, causing incorrect label placement in rendered charts.

Useful? React with 👍 / 👎.

categories = _list(overlay.get("categories"))
totals = _float_or_none_list(overlay.get("totals"))
total_label_tops = _float_or_none_list(overlay.get("total_label_tops"))
series_names = _string_list(overlay.get("series_names"))

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep non-string series names instead of dropping them

Filtering series_names through _string_list drops previously accepted non-string names (for example YAML name: 2024, parsed as an int). Earlier code rendered these names with str(name) in legend/label paths, but now they are removed before use, which can make series_names empty (skipping legend rendering entirely) and break name-based segment selectors for those series.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant