Skip to content

feat: upgrade to ggsql 0.5.2 with native SVG/PDF/HEP writers - #9

Merged
cpsievert merged 17 commits into
mainfrom
ggsql-0.5.2-upgrade
Oct 12, 2026
Merged

cpsievert merged 17 commits into
mainfrom
ggsql-0.5.2-upgrade

Conversation

@cpsievert

@cpsievert cpsievert commented Oct 9, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Upgrades the Python bindings from ggsql Rust crate v0.3.3 to v0.5.2, and exposes the crate's new native rendering capabilities in Python. Upstream 0.4.0–0.5.2 brings many ggsql language and rendering improvements — see the crate changelog for details.

High-level impact:

  • Native rendering, no JavaScript: new SVGWriter, PDFWriter, and HepWriter classes render Specs directly via ggsql's native renderer (hephaestus). render() returns str (SVG) or bytes (PDF/HEP), and render warnings are surfaced via warnings.warn().
  • One-call file export: new ggsql.save(spec, file, ...) helper picks the writer from the file extension (.svg, .pdf, .hep, or .json for Vega-Lite).
  • Builtin datasets: the crate's builtin-data feature is enabled, so queries can reference datasets like ggsql:world.
  • Explicit chart kwargs: VegaLiteWriter.render_chart() and render_altair() now accept validate, height, and width as keyword arguments (height/width default to None, preserving ggsql's dimensions; validate defaults to False, same behavior as before).
  • Dependency bumps: Arrow v56 → v58; Python package version bumped to 0.5.2 to match the crate.

Also documents the py/v* release-tag workflow in CLAUDE.md.

Note: Windows jobs pinned to windows-2022 (temporary)

windows-latest now resolves to a Windows Server 2025 image with the VS2026/v145 toolchain, whose STL removed stdext::checked_array_iterator. ggsql 0.5.2 pins duckdb ~1.10502, and duckdb 1.5.2's vendored fmt still uses that symbol under #ifdef _SECURE_SCL, so the bundled duckdb build hard-fails on windows-latest (duckdb/duckdb-rs#783, fixed in libduckdb-sys 1.10506.0). Upstream ggsql doesn't hit this today because its PR CI is ubuntu-only and its release builds already pin windows-2022, whose VS2022/v143 STL still carries the deprecated symbol.

This PR therefore pins both Windows workflows (python.yml, release-python.yml) to windows-2022 as a stopgap, with in-file comments explaining why. Follow-up: once a ggsql release depends on duckdb ~1.10506, bump the ggsql dependency and revert both pins to windows-latest.

Verification

import ggsql

reader = ggsql.DuckDBReader("duckdb://memory")
spec = reader.execute("SELECT 1 AS x, 2 AS y VISUALISE x, y DRAW point")

svg = ggsql.SVGWriter(width=600, height=400).render(spec)  # str
pdf = ggsql.PDFWriter(width=600, height=400).render(spec)  # bytes

ggsql.save(spec, "plot.svg")
ggsql.save(spec, "plot.pdf")
ggsql.save(spec, "plot.json")  # Vega-Lite JSON

Tests: uv run pytest tests/ -v (includes new coverage for the native writers, save(), and builtin datasets). Rust checks: cargo fmt -- --check, cargo clippy -- -D warnings.

Closes #8

@cpsievert
cpsievert marked this pull request as ready for review October 9, 2026 15:38
@cpsievert
cpsievert requested a balanced review from Copilot October 9, 2026 15:38

This comment was marked as resolved.

@cpsievert

Copy link
Copy Markdown
Collaborator Author

Copilot review findings addressed in c6a8ce6:

  1. MSRV — bumped rust-version to 1.88, which the native-writer dependency graph (parley 0.9.0) requires.
  2. DPI validation — SVGWriter/PDFWriter constructors (and thus ggsql.save()) now raise ValueError for non-positive or non-finite dpi.
  3. Builtin-data coverage — added integration tests executing ggsql:penguins and ggsql:airquality (including an end-to-end SVG render), plus dpi validation tests. Test count: 64 → 77.

@cpsievert
cpsievert force-pushed the ggsql-0.5.2-upgrade branch from c6a8ce6 to a2ddf17 Compare October 12, 2026 15:22
- Bump declared Rust MSRV to 1.88: the native-writer dependency graph (parley 0.9) requires it, so 1.86 was not actually supported.

- Validate dpi in SVGWriter/PDFWriter constructors (rejects non-positive and non-finite values) instead of letting them reach the renderer.

- Add Python integration tests for builtin datasets (ggsql:penguins, ggsql:airquality) and dpi validation.
windows-latest now resolves to a Windows Server 2025 image with the VS2026/v145 toolchain, whose STL removed stdext::checked_array_iterator. ggsql 0.5.2 pins duckdb ~1.10502, and duckdb 1.5.2's vendored fmt still uses that symbol under #ifdef _SECURE_SCL, so the bundled duckdb build hard-fails on windows-latest (duckdb/duckdb-rs#783, fixed upstream in libduckdb-sys 1.10506).

windows-2022's VS2022/v143 STL still has the (deprecated) symbol, and upstream ggsql pins windows-2022 for its own release builds for the same reason. This is a stopgap: revert to windows-latest once ggsql releases with duckdb ~1.10506.
@cpsievert
cpsievert force-pushed the ggsql-0.5.2-upgrade branch from a2ddf17 to 75e5967 Compare October 12, 2026 15:54
The 0.3.3 release went out with its changelog entries still under
UNRELEASED (and the altair.Chart fallback entry misfiled under 0.3.2).
Record them under a proper 0.3.3 section, and add a release checklist
to CLAUDE.md so the changelog/version steps aren't skipped again.
ggsql.__version__ was hardcoded and drifted from pyproject (it read
0.3.0 in the published 0.3.1-0.3.3 wheels). Read it via
importlib.metadata instead, with a test pinning it to pyproject.toml.

Also reject non-positive width/height in SVGWriter/PDFWriter/save()
with a clear ValueError, matching the existing dpi validation, instead
of surfacing a raw PyO3 OverflowError (negative) or silently clamping
(zero).

This comment was marked as resolved.

SVG/PDF/HEP rendering is pure Rust work on immutable data, but it ran
while holding the GIL, blocking other Python threads for the duration.
Wrap the render calls in Python::detach so concurrent Python code (e.g.
a web server or async event loop) keeps running during a slow render.

Safety is compiler-checked: detach requires a Send closure, so this
only builds because the writer and spec types are Sync. Single-threaded
behavior is unchanged.
The crate's native writers take more than width/height/dpi: units,
background, and per-format settings (svg: text/embed-fonts/id-prefix;
pdf: compress/links; hep: lossy/embed-fonts). Route constructor kwargs
through the crate's WriterOptions/from_options so ggsql itself validates
keys per writer — an unknown key or unusable value raises ValueError
naming the option and listing the supported ones. Python booleans become
"true"/"false"; underscore spellings match the crate's hyphenated keys
via its key normalisation.

This goes beyond the R bindings' surface (width/height/dpi only) by
design: routing through from_options means options added upstream become
available here without binding changes.

HepWriter also gains optional width/height/dpi, recorded as consumer
hints only when given (an optionless writer still records no hint).
save() passes **options through to the native writer.
…eel smoke test and writer warning tests

- README: document that native SVG/PDF/HEP writers dlopen libfontconfig.so.1
  on Linux and need installed fonts; renders silently lose text otherwise
- CI: smoke-test the built wheel in a python:3.12-slim container
  provisioned per the README, asserting rendered SVG retains its text
- tests: exercise the render-issue warning path with a stubbed inner
  writer returning non-empty issues (pytest.warns + output preserved),
  plus a no-warning case for an empty issue list
.png/.jpg/.jpeg/.tif/.tiff/.webp now raise a targeted ValueError
explaining that ggsql's raster writers need a GPU adapter (unavailable
in some environments) and suggesting .svg/.pdf plus a converter of the
user's choice, instead of the generic unsupported-extension message
they previously shared with typos. The GPU-backed raster writers stay
disabled; this only improves the error UX.
SVGWriter/PDFWriter show their constructor dimensions; HepWriter and
VegaLiteWriter show the bare call. Pure Python-layer change on the
public wrappers.
@cpsievert
cpsievert added this pull request to stack #11 October 12, 2026 21:40
@cpsievert
cpsievert force-pushed the ggsql-0.5.2-upgrade branch from 28f44d2 to a55d36b Compare October 12, 2026 22:20
@cpsievert
cpsievert merged commit cfe47e7 into main Oct 12, 2026
29 checks passed
cpsievert added a commit that referenced this pull request Oct 12, 2026
* test: failing tests for CachingReader binding

* feat: add CachingReader binding (duckdb-string and bridged primaries)

Wraps upstream ggsql::reader::CachingReader: an internal in-memory
DuckDB cache backend, primary given as a duckdb:// connection string
(built fresh internally) or any Python object with the reader bridge
protocol. Config from GGSQL_CACHE_* env vars with keyword overrides.
Package-level export included here rather than as a later task because
the constructor tests reference ggsql.CachingReader directly.

* chore: cargo fmt for CachingReader binding

* test: CachingReader memoization, TTL, and disable paths

* test: CachingReader layer-source and cache-routing scenarios

* test: export check for CachingReader; hoist _CountingReader to module scope

* docs: clarify enabled=False only disables memoization

Registered tables still live in the private cache, reads referencing
them still route there, and derived SQL still runs on the cache backend
— the flag only stops the memo being consulted or written. Addresses
Copilot review on #10.

* Add CHANGELOG entry for CachingReader (#10)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update ggsql dependency to v0.4.1

2 participants