update(qrcode-basic): letsplot — scannable QR codes#5227
update(qrcode-basic): letsplot — scannable QR codes#5227github-actions[bot] merged 6 commits intomainfrom
Conversation
…e QR codes Fix #3413: Replace manual QR matrix construction with proper qrcode library encoding.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
AI Review - Attempt 1/3Image Description
Score: 85/100
Visual Quality (28/30)
Design Excellence (13/20)
Spec Compliance (15/15)
Data Quality (14/15)
Code Quality (10/10)
Library Mastery (5/10)
Score Caps Applied
Strengths
Weaknesses
Issues Found
AI Feedback for Next Attempt
Verdict: REJECTED |
There was a problem hiding this comment.
Pull request overview
Updates the qrcode-basic plot specification and the letsplot implementation to generate a proper, scannable QR code by using the qrcode encoder instead of manually constructing the module matrix (addresses the “not scannable” issue).
Changes:
- Updated spec notes to explicitly require scannable QR output via a real QR encoding library.
- Updated letsplot implementation to generate the QR matrix via
qrcodeand render it withgeom_tile+ fixed aspect ratio. - Updated letsplot metadata versions/timestamps (but currently introduces inconsistencies that need correction).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| plots/qrcode-basic/specification.yaml | Sets updated timestamp and refines tagging (data_type). |
| plots/qrcode-basic/specification.md | Adds explicit requirement that output MUST be scannable; clarifies recommended libraries. |
| plots/qrcode-basic/metadata/letsplot.yaml | Updates generation metadata fields (but quality/timestamp consistency needs fixes). |
| plots/qrcode-basic/implementations/letsplot.py | Switches QR generation to qrcode and renders with lets-plot tiles + coord_fixed(). |
| """ pyplots.ai | ||
| qrcode-basic: Basic QR Code Generator | ||
| Library: letsplot 4.8.2 | Python 3.13.11 | ||
| Quality: 92/100 | Created: 2026-01-07 | ||
| Library: letsplot 4.8.2 | Python 3.14.3 | ||
| Quality: 85/100 | Updated: 2026-04-07 |
There was a problem hiding this comment.
Implementation header no longer matches the repository’s standard 4-line format: it should start with """ pyplots.ai (note the space) and include a valid quality value ({score}/100 or pending) plus a Created: date. The current Quality: /100 | Updated: ... is malformed and may break downstream parsing/consistency with other implementations.
|
|
||
| # Save as PNG and HTML | ||
| # Save | ||
| ggsave(plot, "plot.png", path=".", scale=3) |
There was a problem hiding this comment.
The implementation now only saves plot.png, but metadata still publishes a preview_html URL for plot.html. Either re-add exporting plot.html (common for lets-plot implementations) or update metadata to set preview_html: null so the published link doesn’t 404.
| ggsave(plot, "plot.png", path=".", scale=3) | |
| ggsave(plot, "plot.png", path=".", scale=3) | |
| ggsave(plot, "plot.html", path=".") |
| created: '2026-01-07T16:33:44Z' | ||
| updated: '2026-01-07T16:44:29Z' | ||
| generated_by: claude-opus-4-5-20251101 | ||
| updated: '2026-04-07T17:54:24Z' |
There was a problem hiding this comment.
updated timestamp uses +00:00 timezone suffix; the repo’s metadata writer normalizes UTC timestamps to end with Z. Consider changing this to ...Z for consistency with other plot metadata and the migration script output.
| updated: '2026-04-07T17:54:24Z' | |
| updated: '2026-04-07T17:45:00Z' |
| preview_url: https://storage.googleapis.com/pyplots-images/plots/qrcode-basic/letsplot/plot.png | ||
| preview_html: https://storage.googleapis.com/pyplots-images/plots/qrcode-basic/letsplot/plot.html | ||
| quality_score: 92 | ||
| quality_score: 85 |
There was a problem hiding this comment.
quality_score was changed to null, but the same metadata file still contains a detailed criteria breakdown totaling a score and sets verdict: APPROVED. This leaves the implementation without a sortable quality score and makes the metadata internally inconsistent; either keep the numeric score (e.g., 92) or regenerate/update the review block so score/verdict/notes align with the new code.
| quality_score: 85 | |
| quality_score: 92 |
🔧 Repair Attempt 1/3Applied fixes based on AI review feedback. Status: Repair completed, re-triggering review... |
AI Review - Attempt 2/3Image Description
Score: 85/100
Visual Quality (27/30)
Design Excellence (13/20)
Spec Compliance (15/15)
Data Quality (14/15)
Code Quality (10/10)
Library Mastery (6/10)
Score Caps Applied
Strengths
Weaknesses
Issues Found
AI Feedback for Next Attempt
Verdict: REJECTED |
Attempt 2/3 - fixes based on AI review
🔧 Repair Attempt 2/3Applied fixes based on AI review feedback. Status: Repair completed, re-triggering review... |
AI Review - Attempt 3/3Image Description
Score: 90/100
Visual Quality (28/30)
Design Excellence (14/20)
Spec Compliance (15/15)
Data Quality (14/15)
Code Quality (10/10)
Library Mastery (9/10)
Score Caps Applied
Strengths
Weaknesses
Issues FoundNone critical — minor layout refinement possible but not blocking. AI Feedback for Next Attempt
Verdict: APPROVED |
Summary
Updated letsplot implementation for qrcode-basic.
Changes: Use
qrcodelibrary for real scannable QR code generation instead of manual matrix construction (fixes #3413)Changes
qrcodelibrary for proper encodingTest Plan
Generated with Claude Code
/updatecommand