Repository navigation
feat: upgrade to ggsql 0.5.2 with native SVG/PDF/HEP writers - #9
Merged
Merged
Conversation
Collaborator
Author
|
Copilot review findings addressed in c6a8ce6:
|
Test expectation update: point's position aesthetics became optional in ggsql 0.4.0, so the missing-required-aesthetic test now uses a text layer missing its required label aesthetic.
cpsievert
force-pushed
the
ggsql-0.5.2-upgrade
branch
from
October 12, 2026 15:22
c6a8ce6 to
a2ddf17
Compare
- 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
force-pushed
the
ggsql-0.5.2-upgrade
branch
from
October 12, 2026 15:54
a2ddf17 to
75e5967
Compare
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).
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.
…e writers" This reverts commit 97c5a28.
…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
added this pull request to stack #11
October 12, 2026 21:40
cpsievert
force-pushed
the
ggsql-0.5.2-upgrade
branch
from
October 12, 2026 22:20
28f44d2 to
a55d36b
Compare
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)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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:
SVGWriter,PDFWriter, andHepWriterclasses renderSpecs directly via ggsql's native renderer (hephaestus).render()returnsstr(SVG) orbytes(PDF/HEP), and render warnings are surfaced viawarnings.warn().ggsql.save(spec, file, ...)helper picks the writer from the file extension (.svg,.pdf,.hep, or.jsonfor Vega-Lite).builtin-datafeature is enabled, so queries can reference datasets likeggsql:world.VegaLiteWriter.render_chart()andrender_altair()now acceptvalidate,height, andwidthas keyword arguments (height/widthdefault toNone, preserving ggsql's dimensions;validatedefaults toFalse, same behavior as before).Also documents the
py/v*release-tag workflow inCLAUDE.md.Note: Windows jobs pinned to
windows-2022(temporary)windows-latestnow resolves to a Windows Server 2025 image with the VS2026/v145 toolchain, whose STL removedstdext::checked_array_iterator. ggsql 0.5.2 pinsduckdb ~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 onwindows-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 pinwindows-2022, whose VS2022/v143 STL still carries the deprecated symbol.This PR therefore pins both Windows workflows (
python.yml,release-python.yml) towindows-2022as a stopgap, with in-file comments explaining why. Follow-up: once a ggsql release depends onduckdb ~1.10506, bump the ggsql dependency and revert both pins towindows-latest.Verification
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