Repository navigation
feat(charts): generalize the Python chart example with brand themes - #165
AsharibAli wants to merge 11 commits into
Conversation
ConalMullan
left a comment
There was a problem hiding this comment.
Thanks. Turning examples/data-viz-chart (Matplotlib for the data, deterministic text on top) into something that works with any dataset is useful, and it builds on a pattern the toolkit already documents. To fit it in:
- Generalise the example rather than adding a separate tool. Either give
examples/data-viz-chart/build.pya--spec chart.jsonoption, or have the example call this tool, so there's one implementation of the pattern, not two. If it stays a tool, the example should use it. - Drop the derived narration. Text generated from the values ("X rose to Y") reads as filler. The script should come from the author. Recording the dataset hash is fine.
- Take brand colours and fonts with
--brand <name>(frombrands/<name>/brand.json) instead of Matplotlib defaults. - Keep titles, source and headline numbers as overlay text, as the example does, rather than as Matplotlib text.
tools/media_ops.pycomes in once via #162. Run by default (no--run), overwrite outputs, temp file +os.replace, and document it in CLAUDE.md and the registry entry instead ofdocs/data-video.md.
Separately, a Remotion-native <DataChart> component (brand-themed, revealed on narration cues) is going on our roadmap. That's a different piece of work and doesn't block this one.
|
@ConalMullan, I addressed your review in
All seven focused tests passed. I also rendered and visually inspected the full 15-second, 1920x1080 bundled example. The existing Remotion DataChart implementation on main is unchanged. The five updated branches merge together without conflicts. Combined verification ran 153 Python tests, with 151 passing and two POSIX process tests skipped on Windows. The existing undefined-name lint passed. A real combined CLI flow produced a 1080x1920 Short with joined audio, ducked music, timed effects, SRT/VTT and a QC-normalized mix measured at -14.03 LUFS. |
ConalMullan
left a comment
There was a problem hiding this comment.
Thanks for this round. The single renderer, brand loading and author-only narration are just what we hoped for, and the tests pass here. I rendered the bundled line example and a small bar chart to check the frames, and a few things showed up before we merge:
- Line charts are spaced by row, not by date. The star series is sampled every one or two days, so the gaps between ticks vary, and monthly data with a missing month would draw a misleading slope. Please space the x axis by real dates when the period parses as one, with readable month ticks ("Jan 2026") as the old example had.
- Bar charts animate from one period to the next but never show which period is on screen. Please show the current period, or render one period only.
- Each bar takes a different brand colour. Our design rules allow one data accent: please draw bars and single lines in the brand primary, with any other data in one neutral. A single series also doesn't need a legend.
- Could the example get its headline number back (718 stars), with the text fading in at a time set in the spec, as the old build did? With the headline empty there is a wide gap above the chart.
- Small: the moviepy skill (
.claude/skills/moviepy/SKILL.md, lines 25 and 250) still describes the old example, andfontFallbackis always true for the default brand because its first font issystem-ui.
Happy to help if any of this is unclear. The other four PRs are in now (#162 and #163 merged, #161 and #167 merging with small fixes from us), so thanks for sticking with these.
|
@ConalMullan, I addressed this review in
The branch also uses the shared media helpers now available through #162. All 19 focused chart tests passed. The full suite ran 218 tests, with 201 passing and 17 existing platform or optional YouTube dependency skips. I rendered and inspected the full 15-second, 1920x1080 example and a 3-second, 1280x720 multi-period bar chart. Decoded MP4 frames also confirmed the headline was hidden before its start, partially visible during the fade and fully visible afterward. Undefined-name lint and copy lint passed. GitHub's unit-test job is still installing FFmpeg. |
The existing Python chart example now accepts CSV/JSON specs through one shared renderer. Matplotlib draws the data, and Pillow overlays author-written text through MoviePy.
Jan 2026. Missing months keep their true gap. Categorical labels retain equal spacing.718 starsheadline.headlineStartSeconds: 10andheadlineFadeSeconds: 0.5control its reveal. Validate timing against the final duration, including narration-driven duration.--brand. Resolve generic families such assystem-uiwithout falsely reporting a missing font. Record actual font selection and fallback when a requested family is unavailable.media_ops.pynow on main for validation, hashing and atomic report writes.--dry-run. UpdateCLAUDE.md, the registry, example README and both MoviePy skill references.Validation passed all 19 focused chart tests. The full Python suite ran 218 tests, with 201 passing and 17 skipped for POSIX-only checks or the uninstalled optional YouTube extra. Undefined-name lint and copy lint passed. The complete bundled example rendered at 1920x1080 for 15 seconds, and a multi-period bar chart rendered at 1280x720 for 3 seconds. Decoded frames verified the headline fade and were visually inspected for dates, colours and period text.
This uses the existing Matplotlib, Pillow, MoviePy and FFmpeg dependencies. No hosted service, model download or paid inference is added.
Feature inventory