Skip to content

feat(letsplot): implement ecdf-basic#5362

Merged
github-actions[bot] merged 3 commits intomainfrom
implementation/ecdf-basic/letsplot
Apr 24, 2026
Merged

feat(letsplot): implement ecdf-basic#5362
github-actions[bot] merged 3 commits intomainfrom
implementation/ecdf-basic/letsplot

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

Implementation: ecdf-basic - python/letsplot

Implements the python/letsplot version of ecdf-basic.

File: plots/ecdf-basic/implementations/python/letsplot.py

Parent Issue: #976


🤖 impl-generate workflow

Regen from quality 96. Addressed: VQ-07 grid too subtle -> solid grid tuned to each theme (C9C7C1 light / 565551 dark, size 0.6) for easier percentile reading; added ANYPLOT_THEME theme tokens and plot-{THEME}.{png,html} outputs; fixed brand color (306998 -> Okabe-Ito 009E73); title pyplots.ai -> anyplot.ai; replaced manual ECDF calc with stat_ecdf(geom=step).
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Apr 24, 2026

AI Review - Attempt 1/3

Image Description

Light render (plot-light.png): The plot renders on a warm off-white background (~#FAF8F1) — correct for the light theme. The title "Web Service Response Times · ecdf-basic · letsplot · anyplot.ai" appears in dark ink at the top-left. The x-axis label "Response Time (ms)" and y-axis label "Cumulative Proportion" are clearly readable in dark ink. Tick labels at 0, 0.25, 0.5, 0.75, 1.0 (y) and 0–270 (x) are visible in secondary dark-soft ink. The ECDF step curve is rendered in brand green #009E73 and shows a distinctive bimodal shape: a fast initial rise (exponential component peaking around 20–40ms) followed by a plateau then a second rise (normal component around 150–230ms). Both horizontal and vertical grid lines are present in medium warm gray — visible but slightly more opaque than ideal. All text is clearly readable against the light background. Legibility verdict: PASS.

Dark render (plot-dark.png): The plot renders on a deep warm near-black background (~#1A1A17) — correct for the dark theme. The title and axis labels appear in light off-white (#F0EFE8) and tick labels in light secondary ink (#B8B7B0). The data series is the identical #009E73 green — data colors unchanged from the light render, confirming only chrome flipped. Grid lines appear as muted warm gray (#565551), more subtle than in the light render but still visible. No dark-on-dark failures detected: no black labels on near-black background. Legibility verdict: PASS.

Both paragraphs are required. A review that only describes one render is invalid.

Score: 87/100

Category Score Max
Visual Quality 30 30
Design Excellence 10 20
Spec Compliance 14 15
Data Quality 15 15
Code Quality 10 10
Library Mastery 8 10
Total 87 100

Visual Quality (30/30)

  • VQ-01: Text Legibility (8/8) — All sizes explicitly set: 24pt title, 20pt axis labels, 16pt ticks. Readable in both themes.
  • VQ-02: No Overlap (6/6) — No overlapping elements in either render.
  • VQ-03: Element Visibility (6/6) — ECDF step line clearly visible with size=2 scaled to 6px.
  • VQ-04: Color Accessibility (2/2) — Single series in #009E73, CVD-safe, good contrast on both surfaces.
  • VQ-05: Layout & Canvas (4/4) — Plot fills canvas well, balanced margins, axes scaled to data range.
  • VQ-06: Axis Labels & Title (2/2) — "Response Time (ms)" and "Cumulative Proportion" are descriptive with appropriate units.
  • VQ-07: Palette Compliance (2/2) — #009E73 first series, #FAF8F1 light / #1A1A17 dark backgrounds, chrome fully adaptive.

Design Excellence (10/20)

  • DE-01: Aesthetic Sophistication (4/8) — Well-configured default appearance with correct colors and clean layout, but not exceptional. No typographic hierarchy, no focal emphasis on the bimodal insight. Looks like a polished library default, not publication-quality.
  • DE-02: Visual Refinement (4/6) — Some refinement present: minor grids removed, axis lines explicitly styled, custom GRID color. However, the grid color (#C9C7C1 light / #565551 dark) is ~2.5x more opaque than the style guide's recommended rgba(26,26,23,0.10), making the grid compete with data rather than recede.
  • DE-03: Data Storytelling (2/6) — Data displayed but not interpreted. The bimodal ECDF shape (exponential fast-path + normal slow-path) reveals two distinct response-time populations — a meaningful insight — but no visual emphasis, annotations, or percentile markers help the viewer find this story.

Spec Compliance (14/15)

  • SC-01: Plot Type (5/5) — Correct ECDF step function via stat_ecdf(geom='step').
  • SC-02: Required Features (4/4) — Step function, y-axis [0,1] with quartile breaks, distinct line style, grid lines for percentile reading. All present.
  • SC-03: Data Mapping (3/3) — X = response_time, Y = cumulative proportion. Correct.
  • SC-04: Title & Legend (2/3) — Title is "Web Service Response Times · ecdf-basic · letsplot · anyplot.ai" — the extra prefix makes it a 4-part title rather than the prescribed 3-part format ecdf-basic · letsplot · anyplot.ai. No legend needed for single series (N/A, correct omission).

Data Quality (15/15)

  • DQ-01: Feature Coverage (6/6) — Bimodal distribution showcases ECDF strengths: non-parametric estimation, percentile reading, distribution shape without binning.
  • DQ-02: Realistic Context (5/5) — Web service response times with realistic mixed distribution (exponential fast-path + normal slow-path). Neutral, professional scenario.
  • DQ-03: Appropriate Scale (4/4) — 0–270ms range is realistic; 200 observations within the 50–500 spec recommendation.

Code Quality (10/10)

  • CQ-01: KISS Structure (3/3) — Clean: imports → tokens → data → plot → save. No functions or classes.
  • CQ-02: Reproducibility (2/2) — np.random.seed(42) set.
  • CQ-03: Clean Imports (2/2) — All imports used; no unused dependencies.
  • CQ-04: Code Elegance (2/2) — Clean, Pythonic, appropriate complexity.
  • CQ-05: Output & API (1/1) — Saves plot-{THEME}.png and plot-{THEME}.html with scale=3. Current API.

Library Mastery (8/10)

  • LM-01: Idiomatic Usage (5/5) — Grammar-of-graphics approach with stat_ecdf statistical layer. Idiomatic lets-plot/ggplot2 pattern.
  • LM-02: Distinctive Features (3/5) — stat_ecdf is shared with plotnine; HTML export is lets-plot specific but is just boilerplate. Could leverage lets-plot's interactive tooltip customization (layer_tooltips) or other lets-plot-specific features more distinctively.

Score Caps Applied

  • None applied (all cap conditions clear).

Strengths

  • Correct ECDF via stat_ecdf(geom='step') — idiomatic grammar-of-graphics approach computing the distribution natively
  • Perfect code quality: KISS structure, reproducible seed, clean imports, correct output filenames for both PNG and HTML
  • Realistic data scenario (web service response times with bimodal exponential+normal distribution)
  • Full theme-adaptive chrome: PAGE_BG, INK, INK_SOFT tokens applied correctly across both renders
  • Font sizes explicitly set (24pt/20pt/16pt) matching style guide requirements
  • Brand green #009E73 consistent across both renders, correct palette compliance

Weaknesses

  • Grid lines too prominent: #C9C7C1 / #565551 are too opaque vs. style guide's rgba(26,26,23,0.10) — grid competes with data
  • No data storytelling: bimodal ECDF shape reveals two response-time populations but insight is not emphasized (no percentile markers, no annotation)
  • Title format has extra prefix "Web Service Response Times · " — should be ecdf-basic · letsplot · anyplot.ai
  • LM-02 underutilized: lets-plot's layer_tooltips() or other interactive capabilities not leveraged in the plot layer

Issues Found

  1. DE-02 GRID: Grid lines use #C9C7C1 (light) / #565551 (dark) which are ~2.5x more opaque than the recommended rgba(26,26,23,0.10) RULE token from the style guide.
    • Fix: Replace with panel_grid_major=element_line(color="rgba(26,26,23,0.10)", size=0.5) for light and use the appropriate RULE token for dark, or use a softer hex like #E4E2DC / #3A3A36.
  2. DE-03 LOW: The bimodal ECDF shape is a clear insight (exponential + normal populations) that is completely invisible without emphasis.
    • Fix: Add reference lines at key percentiles (p50, p90) with geom_vline + geom_hline or annotate the "knee" where the two populations join.
  3. SC-04 TITLE: Title format has 4 parts instead of 3.
    • Fix: Change to "ecdf-basic · letsplot · anyplot.ai" (remove the "Web Service Response Times · " prefix).

AI Feedback for Next Attempt

Three targeted fixes will push this to approval: (1) Make the grid more subtle — use rgba(26,26,23,0.10) (light) and rgba(240,239,232,0.10) (dark) for panel_grid_major color instead of the current solid hex grays. (2) Add percentile reference lines at p50 and p90 using geom_hline(yintercept=[0.5, 0.9], linetype='dashed', color=INK_SOFT, size=0.8) with matching geom_vline at the corresponding x values — this gives the viewer an immediate reading of median and 90th-percentile response time without annotations. (3) Fix the title to the exact 3-part format: "ecdf-basic · letsplot · anyplot.ai". These changes together should lift DE-02 from 4→5, DE-03 from 2→4, and SC-04 from 2→3, bringing the total to ~93.

Verdict: REJECTED

@github-actions github-actions Bot added quality:87 Quality score 87/100 ai-approved Quality OK, ready for merge labels Apr 24, 2026
@github-actions github-actions Bot merged commit 31cee04 into main Apr 24, 2026
3 checks passed
@github-actions github-actions Bot deleted the implementation/ecdf-basic/letsplot branch April 24, 2026 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-approved Quality OK, ready for merge quality:87 Quality score 87/100

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants