refactor(chart-engine): complete strict typing cleanup pass #24
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 09215957b3
ℹ️ 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".
| labels.font.size = font_size | ||
| else: | ||
| labels.font.size = Pt(float(font_size)) | ||
| labels.font.size = Pt(float(_int(font_size, int(DEFAULT_BAR_DATA_LABEL_FONT_SIZE)))) |
There was a problem hiding this comment.
apply_data_label_style now falls back via Pt(float(_int(font_size, int(DEFAULT_BAR_DATA_LABEL_FONT_SIZE)))), but int(DEFAULT_BAR_DATA_LABEL_FONT_SIZE) is an EMU value, not points. When data_labels.font_size is present but not an Pt instance (for example a quoted numeric like "14"), this path produces a massive point size instead of the intended default/parsed size, which can make labels overflow or render unusably large.
Useful? React with 👍 / 👎.
| if series_selector: | ||
| series_names = [series.name for series in chart.series] | ||
|
|
||
| if series_selector is not None: |
There was a problem hiding this comment.
The selector gate changed from a truthy check to is not None, so configs like data_labels: {series_indices: []} no longer fall back to plot-level labels and instead apply labels to zero series. This is a behavior regression from the previous implementation and can silently drop all data labels when templating emits an empty selector list.
Useful? React with 👍 / 👎.
1 participant
Summary
Complete the remaining chart-engine typing cleanup by removing all file-level pyright suppression headers and replacing dynamic access with explicit typed coercion/wrappers.
This continues the incremental overlay/refactor series and finishes the suppression-reduction pass across chart-engine core modules.
Commits in this PR
refactor(chart-engine): type chart style and template helpersrefactor(chart-engine): tighten typing for chart builder orchestrationrefactor(chart-engine): tighten typing for chart payload buildersrefactor(chart-engine): tighten typing for annotation helperstest(charts): narrow payload style mapping types in assertionsdocs(chart-engine): update typing cleanup roadmapMain changes
Removed remaining broad pyright suppression headers from:
clean_slides/chart_engine/style.pyclean_slides/chart_engine/text_templates.pyclean_slides/chart_engine/template_ops.pyclean_slides/chart_engine/overlays.pyclean_slides/chart_engine/builder.pyclean_slides/chart_engine/payloads.pyclean_slides/chart_engine/annotations.pyAdded typed coercion/normalization helpers where needed (
mapping/list/number/color/base_dirparsing) to avoid unknown-type propagation while preserving behavior.Introduced typed callable boundaries in
builder.pyfor dynamic module entrypoints (add_waterfall_title, payload builders).Kept existing chart behavior while tightening internals:
Updated tests to narrow payload-style mapping access under strict pyright.
Updated
clean_slides/chart_engine/README.mdroadmap text now that suppression cleanup is complete.Validation
.venv/bin/pyright.venv/bin/pytest -q.venv/bin/pre-commit run --all-files