Add Static/Dynamic layout viewer tabs to cells page#670
Conversation
Replace the single plot directive in the API Reference section with sphinx-design tab-set showing a Static PNG tab and a Dynamic kwasm viewer tab for each cell. Gallery grid cards retain plot directives for dynamic thumbnail rendering. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
nikosavola
requested review from
Alex-l-r,
ThomasPluck,
cdaunt,
joamatab,
sheron-lian and
thanojo
as code owners
Reviewer's GuideAdds kwasm-based static/dynamic layout viewers to the cells API reference page by pre-generating GDS/PNG artifacts and wiring them into a tabbed Sphinx template, while refactoring and hardening the GDS/PNG generation logic in the write_cells script. Flow diagram for generating GDS/PNG artifacts and tabbed cells layoutflowchart LR
write_cells[write_cells.py main] --> setup_kwasm[_setup_kwasm_viewer]
write_cells --> loop_cells[for each cell name]
loop_cells --> write_gds[_write_gds]
write_gds --> gen_artifacts[_generate_artifacts]
gen_artifacts --> files[GDS and PNG in kwasm_gds]
write_gds --> has_gds[has_gds flag in cells_items]
has_gds --> template[cells.rst.j2]
template -->|has_gds true| tabset[.. tab-set with Static PNG and Dynamic kwasm iframe]
template -->|has_gds false| plot[.. plot:: fallback]
tabset --> cells_page[cells API reference page]
plot --> cells_page
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
github-actions
Bot
added
documentation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- Consider avoiding the use of the private kwasm.embed._read_artifacts API in _setup_kwasm_viewer and instead expose or wrap a public helper so the script doesn’t rely on internal implementation details.
- In _write_gds, printing full tracebacks on any Exception may be too noisy for normal docs generation; you might want to log a concise error message or restrict exceptions to expected failure modes instead of traceback.print_exc().
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider avoiding the use of the private kwasm.embed._read_artifacts API in _setup_kwasm_viewer and instead expose or wrap a public helper so the script doesn’t rely on internal implementation details.
- In _write_gds, printing full tracebacks on any Exception may be too noisy for normal docs generation; you might want to log a concise error message or restrict exceptions to expected failure modes instead of traceback.print_exc().
## Individual Comments
### Comment 1
<location path=".github/write_cells.py" line_range="10-18" />
<code_context>
from pathlib import Path
+import kwasm.embed
+import matplotlib as mpl
+import matplotlib.pyplot as plt
from gdsfactory.serialization import clean_value_json
from jinja2 import Environment, FileSystemLoader
import qpdk
from qpdk.config import PATH
+mpl.use("Agg")
+
filepath_cells = PATH.docs / "cells.rst"
</code_context>
<issue_to_address>
**issue (bug_risk):** Set the Matplotlib backend before importing pyplot to ensure it takes effect
Because the backend is fixed when `matplotlib.pyplot` is first imported, calling `mpl.use("Agg")` after `import matplotlib.pyplot as plt` is ineffective and can cause issues in headless/CI runs. Please move `mpl.use("Agg")` to immediately after `import matplotlib as mpl` and before importing `pyplot`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| import matplotlib as mpl | ||
| import matplotlib.pyplot as plt | ||
| from gdsfactory.serialization import clean_value_json | ||
| from jinja2 import Environment, FileSystemLoader | ||
|
|
||
| import qpdk | ||
| from qpdk.config import PATH | ||
|
|
||
| mpl.use("Agg") |
There was a problem hiding this comment.
issue (bug_risk): Set the Matplotlib backend before importing pyplot to ensure it takes effect
Because the backend is fixed when matplotlib.pyplot is first imported, calling mpl.use("Agg") after import matplotlib.pyplot as plt is ineffective and can cause issues in headless/CI runs. Please move mpl.use("Agg") to immediately after import matplotlib as mpl and before importing pyplot.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces automated GDS and PNG artifact generation for PDK cells, setting up an interactive kwasm HTML viewer and updating the documentation templates to display static and dynamic tabs for cell previews. Additionally, several test data CSV files have been converted from Git LFS pointers to actual data files. The review feedback highlights two key improvements: avoiding the use of the private API _read_artifacts from kwasm.embed to prevent future breakage, and wrapping the matplotlib figure generation in a try...finally block to guarantee the figure is closed and avoid memory leaks if an exception occurs.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| viewer_path = kwasm_dir / "viewer.html" | ||
| if viewer_path.exists(): | ||
| return | ||
| template = kwasm.embed._read_artifacts() |
There was a problem hiding this comment.
Using a private/internal API _read_artifacts from the kwasm.embed module is risky because internal functions can change or be removed in future versions without notice. If a public API is available, please use that instead.
| def _generate_artifacts(c, name: str) -> None: | ||
| """Generate GDS and PNG plot for the component.""" | ||
| c.draw_ports() | ||
| c.write_gds(gds_dir / f"{name}.gds") | ||
| fig, ax = plt.subplots() | ||
| c.plot(ax=ax) | ||
| fig.savefig(gds_dir / f"{name}.png", dpi=150, bbox_inches="tight") | ||
| plt.close(fig) |
There was a problem hiding this comment.
In _generate_artifacts, the matplotlib figure is created using plt.subplots() and then closed with plt.close(fig). However, if an exception occurs during c.plot(ax=ax) or fig.savefig(...), plt.close(fig) will not be called, which can lead to memory leaks when generating many artifacts. It is safer to use a try...finally block to ensure the figure is always closed.
| def _generate_artifacts(c, name: str) -> None: | |
| """Generate GDS and PNG plot for the component.""" | |
| c.draw_ports() | |
| c.write_gds(gds_dir / f"{name}.gds") | |
| fig, ax = plt.subplots() | |
| c.plot(ax=ax) | |
| fig.savefig(gds_dir / f"{name}.png", dpi=150, bbox_inches="tight") | |
| plt.close(fig) | |
| def _generate_artifacts(c, name: str) -> None: | |
| """Generate GDS and PNG plot for the component.""" | |
| c.draw_ports() | |
| c.write_gds(gds_dir / f"{name}.gds") | |
| fig, ax = plt.subplots() | |
| try: | |
| c.plot(ax=ax) | |
| fig.savefig(gds_dir / f"{name}.png", dpi=150, bbox_inches="tight") | |
| finally: | |
| plt.close(fig) |
2 participants
Summary
This PR adds static PNG and dynamic kwasm-based viewers for cell layouts in the API reference cells page.
.. plot::for dynamic thumbnail rendering.Also resolves the pre-commit Ruff lint failures in
.github/write_cells.pyby refactoring the GDS/PNG generation into a helper function_generate_artifacts.This PR replaces #643, which was opened from an external fork and was permanently blocked due to the required CodeQL check not running on fork pull requests.
Summary by Sourcery
Add static PNG and dynamic kwasm-based viewer tabs to API reference cell layout docs and centralize GDS/PNG generation in a reusable helper.
Enhancements:
Tests: