From 73546128e215c3cf61390288c907041149f6f292 Mon Sep 17 00:00:00 2001 From: Carson Date: Mon, 2 Mar 2026 15:52:20 -0600 Subject: [PATCH 1/4] feat(python): add Reader protocol for custom reader contract Co-Authored-By: Claude Opus 4.6 --- ggsql-python/python/ggsql/__init__.py | 29 +++++++++++++++++-- ggsql-python/tests/test_ggsql.py | 40 ++++++++++++++++++++++++++- 2 files changed, 66 insertions(+), 3 deletions(-) diff --git a/ggsql-python/python/ggsql/__init__.py b/ggsql-python/python/ggsql/__init__.py index f9356e72..69f84646 100644 --- a/ggsql-python/python/ggsql/__init__.py +++ b/ggsql-python/python/ggsql/__init__.py @@ -1,11 +1,12 @@ from __future__ import annotations import json -from typing import Any, Union +from typing import Any, Protocol, Union, runtime_checkable import altair import narwhals as nw from narwhals.typing import IntoFrame +import polars as pl from ggsql._ggsql import ( DuckDBReader, @@ -22,12 +23,13 @@ "VegaLiteWriter", "Validated", "Spec", + "Reader", # Functions "validate", "execute", "render_altair", ] -__version__ = "0.1.0" +__version__ = "0.1.4" # Type alias for any Altair chart type AltairChart = Union[ @@ -41,6 +43,29 @@ ] +@runtime_checkable +class Reader(Protocol): + """Protocol for ggsql database readers. + + Any object implementing these methods can be used as a reader with + ``ggsql.execute()``. Native readers like ``DuckDBReader`` satisfy + this protocol automatically. + + Required methods + ---------------- + execute_sql(sql: str) -> polars.DataFrame + Execute a SQL query and return results as a polars DataFrame. + register(name: str, df: polars.DataFrame, replace: bool = False) -> None + Register a DataFrame as a named table for SQL queries. + """ + + def execute_sql(self, sql: str) -> pl.DataFrame: ... + + def register( + self, name: str, df: pl.DataFrame, replace: bool = False + ) -> None: ... + + def _json_to_altair_chart(vegalite_json: str, **kwargs: Any) -> AltairChart: """Convert a Vega-Lite JSON string to the appropriate Altair chart type.""" spec = json.loads(vegalite_json) diff --git a/ggsql-python/tests/test_ggsql.py b/ggsql-python/tests/test_ggsql.py index fbe4b131..3e38a6f8 100644 --- a/ggsql-python/tests/test_ggsql.py +++ b/ggsql-python/tests/test_ggsql.py @@ -532,6 +532,45 @@ def unregister(self, name: str) -> None: assert "point" in json_output +class TestReaderProtocol: + """Tests for Reader protocol.""" + + def test_duckdb_reader_is_reader(self): + """Native DuckDBReader satisfies the Reader protocol.""" + reader = ggsql.DuckDBReader("duckdb://memory") + assert isinstance(reader, ggsql.Reader) + + def test_custom_reader_is_reader(self): + """Custom reader with correct methods satisfies the Reader protocol.""" + + class MyReader: + def execute_sql(self, sql: str) -> pl.DataFrame: + return pl.DataFrame({"x": [1]}) + + def register( + self, name: str, df: pl.DataFrame, replace: bool = False + ) -> None: + pass + + reader = MyReader() + assert isinstance(reader, ggsql.Reader) + + def test_incomplete_reader_is_not_reader(self): + """Object missing required methods is not a Reader.""" + + class NotAReader: + def execute_sql(self, sql: str) -> pl.DataFrame: + return pl.DataFrame({"x": [1]}) + # Missing register() + + obj = NotAReader() + assert not isinstance(obj, ggsql.Reader) + + def test_reader_is_exported(self): + """Reader is accessible from ggsql module.""" + assert hasattr(ggsql, "Reader") + + class TestVegaLiteWriterRenderChart: """Tests for VegaLiteWriter.render_chart() method.""" @@ -568,4 +607,3 @@ def test_render_chart_facet(self): writer = ggsql.VegaLiteWriter() chart = writer.render_chart(spec, validate=False) assert isinstance(chart, altair.FacetChart) - From 703a246b3263fd4051af84d7ed20dd5048df57fc Mon Sep 17 00:00:00 2001 From: Carson Date: Mon, 2 Mar 2026 16:02:11 -0600 Subject: [PATCH 2/4] feat(python): add typed exception classes (ParseError, ValidationError, ReaderError, WriterError) Co-Authored-By: Claude Opus 4.6 --- ggsql-python/python/ggsql/__init__.py | 9 +++++ ggsql-python/src/lib.rs | 48 ++++++++++++++++++++++----- ggsql-python/tests/test_ggsql.py | 47 ++++++++++++++++++++++++++ 3 files changed, 95 insertions(+), 9 deletions(-) diff --git a/ggsql-python/python/ggsql/__init__.py b/ggsql-python/python/ggsql/__init__.py index 69f84646..42b2338e 100644 --- a/ggsql-python/python/ggsql/__init__.py +++ b/ggsql-python/python/ggsql/__init__.py @@ -15,6 +15,10 @@ Spec, validate, execute, + ParseError, + ValidationError, + ReaderError, + WriterError, ) __all__ = [ @@ -28,6 +32,11 @@ "validate", "execute", "render_altair", + # Exceptions + "ParseError", + "ValidationError", + "ReaderError", + "WriterError", ] __version__ = "0.1.4" diff --git a/ggsql-python/src/lib.rs b/ggsql-python/src/lib.rs index 27d26b68..3afd756b 100644 --- a/ggsql-python/src/lib.rs +++ b/ggsql-python/src/lib.rs @@ -3,6 +3,8 @@ #![allow(clippy::useless_conversion)] use pyo3::prelude::*; +use pyo3::create_exception; +use pyo3::exceptions::PyValueError; use pyo3::types::{PyBytes, PyDict, PyList}; use std::io::Cursor; @@ -12,6 +14,28 @@ use ggsql::validate::{validate as rust_validate, ValidationWarning}; use ggsql::writer::{VegaLiteWriter as RustVegaLiteWriter, Writer as RustWriter}; use ggsql::GgsqlError; +// ============================================================================ +// Custom Exception Classes +// ============================================================================ + +// All subclass ValueError for backwards compatibility +create_exception!(ggsql, ParseError, PyValueError, "Raised on query syntax errors."); +create_exception!(ggsql, ValidationError, PyValueError, "Raised on semantic validation errors."); +create_exception!(ggsql, ReaderError, PyValueError, "Raised on data source errors."); +create_exception!(ggsql, WriterError, PyValueError, "Raised on output generation errors."); + +/// Convert a GgsqlError to the appropriate typed Python exception. +fn ggsql_err_to_py(e: GgsqlError) -> PyErr { + let msg = e.to_string(); + match e { + GgsqlError::ParseError(_) => PyErr::new::(msg), + GgsqlError::ValidationError(_) => PyErr::new::(msg), + GgsqlError::ReaderError(_) => PyErr::new::(msg), + GgsqlError::WriterError(_) => PyErr::new::(msg), + GgsqlError::InternalError(_) => PyErr::new::(msg), + } +} + use polars::prelude::{DataFrame, IpcReader, IpcWriter, SerReader, SerWriter}; // ============================================================================ @@ -175,7 +199,7 @@ macro_rules! try_native_readers { if let Ok(native) = $reader.downcast::<$native_type>() { return native.borrow().inner.execute($query) .map(|s| PySpec { inner: s }) - .map_err(|e| PyErr::new::(e.to_string())); + .map_err(ggsql_err_to_py); } )* }}; @@ -225,7 +249,7 @@ impl PyDuckDBReader { #[new] fn new(connection: &str) -> PyResult { let inner = RustDuckDBReader::from_connection_string(connection) - .map_err(|e| PyErr::new::(e.to_string()))?; + .map_err(ggsql_err_to_py)?; Ok(Self { inner }) } @@ -255,7 +279,7 @@ impl PyDuckDBReader { let rust_df = py_to_polars(py, df)?; self.inner .register(name, rust_df, replace) - .map_err(|e| PyErr::new::(e.to_string())) + .map_err(ggsql_err_to_py) } /// Unregister a previously registered table. @@ -272,7 +296,7 @@ impl PyDuckDBReader { fn unregister(&self, name: &str) -> PyResult<()> { self.inner .unregister(name) - .map_err(|e| PyErr::new::(e.to_string())) + .map_err(ggsql_err_to_py) } /// Execute a SQL query and return the result as a DataFrame. @@ -295,7 +319,7 @@ impl PyDuckDBReader { let df = self .inner .execute_sql(sql) - .map_err(|e| PyErr::new::(e.to_string()))?; + .map_err(ggsql_err_to_py)?; polars_to_py(py, &df) } @@ -330,7 +354,7 @@ impl PyDuckDBReader { self.inner .execute(query) .map(|s| PySpec { inner: s }) - .map_err(|e| PyErr::new::(e.to_string())) + .map_err(ggsql_err_to_py) } } @@ -393,7 +417,7 @@ impl PyVegaLiteWriter { fn render(&self, spec: &PySpec) -> PyResult { self.inner .render(&spec.inner) - .map_err(|e| PyErr::new::(e.to_string())) + .map_err(ggsql_err_to_py) } } @@ -658,7 +682,7 @@ impl PySpec { #[pyfunction] fn validate(query: &str) -> PyResult { let v = rust_validate(query) - .map_err(|e| PyErr::new::(e.to_string()))?; + .map_err(ggsql_err_to_py)?; Ok(PyValidated { sql: v.sql().to_string(), @@ -739,7 +763,7 @@ fn execute(query: &str, reader: &Bound<'_, PyAny>) -> PyResult { bridge .execute(query) .map(|s| PySpec { inner: s }) - .map_err(|e| PyErr::new::(e.to_string())) + .map_err(ggsql_err_to_py) } // ============================================================================ @@ -748,6 +772,12 @@ fn execute(query: &str, reader: &Bound<'_, PyAny>) -> PyResult { #[pymodule] fn _ggsql(m: &Bound<'_, PyModule>) -> PyResult<()> { + // Exceptions + m.add("ParseError", m.py().get_type::())?; + m.add("ValidationError", m.py().get_type::())?; + m.add("ReaderError", m.py().get_type::())?; + m.add("WriterError", m.py().get_type::())?; + // Classes m.add_class::()?; m.add_class::()?; diff --git a/ggsql-python/tests/test_ggsql.py b/ggsql-python/tests/test_ggsql.py index 3e38a6f8..230cea3e 100644 --- a/ggsql-python/tests/test_ggsql.py +++ b/ggsql-python/tests/test_ggsql.py @@ -532,6 +532,53 @@ def unregister(self, name: str) -> None: assert "point" in json_output +class TestExceptions: + """Tests for typed exception classes.""" + + def test_parse_error_on_invalid_syntax(self): + """Invalid syntax raises ParseError when executing.""" + with pytest.raises(ggsql.ParseError): + reader = ggsql.DuckDBReader("duckdb://memory") + reader.execute("SELECT 1 AS x VISUALISE DRAW not_a_geom") + + def test_parse_error_is_value_error(self): + """ParseError is a subclass of ValueError for backwards compat.""" + assert issubclass(ggsql.ParseError, ValueError) + + def test_validation_error_on_missing_aesthetic(self): + """Missing required aesthetic raises ValidationError.""" + with pytest.raises(ggsql.ValidationError): + reader = ggsql.DuckDBReader("duckdb://memory") + reader.execute("SELECT 1 AS x VISUALISE DRAW point MAPPING x AS x") + + def test_validation_error_is_value_error(self): + """ValidationError is a subclass of ValueError for backwards compat.""" + assert issubclass(ggsql.ValidationError, ValueError) + + def test_reader_error_on_bad_sql(self): + """Bad SQL raises ReaderError.""" + with pytest.raises(ggsql.ReaderError): + reader = ggsql.DuckDBReader("duckdb://memory") + reader.execute( + "SELECT * FROM nonexistent_table VISUALISE DRAW point MAPPING x AS x, y AS y" + ) + + def test_reader_error_is_value_error(self): + """ReaderError is a subclass of ValueError for backwards compat.""" + assert issubclass(ggsql.ReaderError, ValueError) + + def test_writer_error_is_value_error(self): + """WriterError is a subclass of ValueError for backwards compat.""" + assert issubclass(ggsql.WriterError, ValueError) + + def test_all_exceptions_exported(self): + """All exception classes are accessible from ggsql module.""" + assert hasattr(ggsql, "ParseError") + assert hasattr(ggsql, "ValidationError") + assert hasattr(ggsql, "ReaderError") + assert hasattr(ggsql, "WriterError") + + class TestReaderProtocol: """Tests for Reader protocol.""" From 3e086c4cbad3a76026e72a1e1c82c975179a1894 Mon Sep 17 00:00:00 2001 From: Carson Date: Mon, 9 Mar 2026 15:45:45 -0500 Subject: [PATCH 3/4] fix(python): pass replace as keyword in PyReaderBridge for protocol consistency The Reader protocol defines replace as a keyword parameter. Make the bridge path match so custom readers aren't required to accept it positionally. Update test custom readers accordingly. Co-Authored-By: Claude Opus 4.6 --- ggsql-python/src/lib.rs | 6 +++++- ggsql-python/tests/test_ggsql.py | 6 +++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/ggsql-python/src/lib.rs b/ggsql-python/src/lib.rs index 3afd756b..a04d2e51 100644 --- a/ggsql-python/src/lib.rs +++ b/ggsql-python/src/lib.rs @@ -166,9 +166,13 @@ impl Reader for PyReaderBridge { Python::attach(|py| { let py_df = polars_to_py(py, &df).map_err(|e| GgsqlError::ReaderError(e.to_string()))?; + let kwargs = PyDict::new(py); + kwargs + .set_item("replace", replace) + .map_err(|e| GgsqlError::ReaderError(e.to_string()))?; self.obj .bind(py) - .call_method1("register", (name, py_df, replace)) + .call_method("register", (name, py_df), Some(&kwargs)) .map_err(|e| GgsqlError::ReaderError(format!("Reader.register() failed: {}", e)))?; Ok(()) }) diff --git a/ggsql-python/tests/test_ggsql.py b/ggsql-python/tests/test_ggsql.py index 230cea3e..962a628d 100644 --- a/ggsql-python/tests/test_ggsql.py +++ b/ggsql-python/tests/test_ggsql.py @@ -402,7 +402,7 @@ def __init__(self): def execute_sql(self, sql: str) -> pl.DataFrame: return self.conn.execute(sql).pl() - def register(self, name: str, df: pl.DataFrame, _replace: bool) -> None: + def register(self, name: str, df: pl.DataFrame, replace: bool = False) -> None: self.conn.register(name, df) reader = RegisterReader() @@ -453,7 +453,7 @@ def __init__(self): def execute_sql(self, sql: str) -> pl.DataFrame: return self.conn.execute(sql).pl() - def register(self, name: str, df: pl.DataFrame, _replace: bool) -> None: + def register(self, name: str, df: pl.DataFrame, replace: bool = False) -> None: self.conn.register(name, df) reader = DuckDBBackedReader() @@ -484,7 +484,7 @@ def execute_sql(self, sql: str) -> pl.DataFrame: self.execute_calls.append(sql) return self.conn.execute(sql).pl() - def register(self, name: str, df: pl.DataFrame, _replace: bool) -> None: + def register(self, name: str, df: pl.DataFrame, replace: bool = False) -> None: self.conn.register(name, df) reader = RecordingReader() From 4214f6e2e5b70f20ef6c05885513c1392e5ac8ad Mon Sep 17 00:00:00 2001 From: Carson Date: Mon, 9 Mar 2026 16:52:21 -0500 Subject: [PATCH 4/4] style: format Rust code Co-Authored-By: Claude Opus 4.6 --- ggsql-python/src/lib.rs | 50 +++++++++++++++++++++++++---------------- 1 file changed, 31 insertions(+), 19 deletions(-) diff --git a/ggsql-python/src/lib.rs b/ggsql-python/src/lib.rs index a04d2e51..91710c21 100644 --- a/ggsql-python/src/lib.rs +++ b/ggsql-python/src/lib.rs @@ -2,9 +2,9 @@ // See: https://github.com/PyO3/pyo3/issues/4327 #![allow(clippy::useless_conversion)] -use pyo3::prelude::*; use pyo3::create_exception; use pyo3::exceptions::PyValueError; +use pyo3::prelude::*; use pyo3::types::{PyBytes, PyDict, PyList}; use std::io::Cursor; @@ -19,10 +19,30 @@ use ggsql::GgsqlError; // ============================================================================ // All subclass ValueError for backwards compatibility -create_exception!(ggsql, ParseError, PyValueError, "Raised on query syntax errors."); -create_exception!(ggsql, ValidationError, PyValueError, "Raised on semantic validation errors."); -create_exception!(ggsql, ReaderError, PyValueError, "Raised on data source errors."); -create_exception!(ggsql, WriterError, PyValueError, "Raised on output generation errors."); +create_exception!( + ggsql, + ParseError, + PyValueError, + "Raised on query syntax errors." +); +create_exception!( + ggsql, + ValidationError, + PyValueError, + "Raised on semantic validation errors." +); +create_exception!( + ggsql, + ReaderError, + PyValueError, + "Raised on data source errors." +); +create_exception!( + ggsql, + WriterError, + PyValueError, + "Raised on output generation errors." +); /// Convert a GgsqlError to the appropriate typed Python exception. fn ggsql_err_to_py(e: GgsqlError) -> PyErr { @@ -252,8 +272,8 @@ impl PyDuckDBReader { /// If the connection string is invalid or the database cannot be opened. #[new] fn new(connection: &str) -> PyResult { - let inner = RustDuckDBReader::from_connection_string(connection) - .map_err(ggsql_err_to_py)?; + let inner = + RustDuckDBReader::from_connection_string(connection).map_err(ggsql_err_to_py)?; Ok(Self { inner }) } @@ -298,9 +318,7 @@ impl PyDuckDBReader { /// ValueError /// If the table wasn't registered via this reader or unregistration fails. fn unregister(&self, name: &str) -> PyResult<()> { - self.inner - .unregister(name) - .map_err(ggsql_err_to_py) + self.inner.unregister(name).map_err(ggsql_err_to_py) } /// Execute a SQL query and return the result as a DataFrame. @@ -320,10 +338,7 @@ impl PyDuckDBReader { /// ValueError /// If the SQL is invalid or execution fails. fn execute_sql(&self, py: Python<'_>, sql: &str) -> PyResult> { - let df = self - .inner - .execute_sql(sql) - .map_err(ggsql_err_to_py)?; + let df = self.inner.execute_sql(sql).map_err(ggsql_err_to_py)?; polars_to_py(py, &df) } @@ -419,9 +434,7 @@ impl PyVegaLiteWriter { /// >>> writer = VegaLiteWriter() /// >>> json_output = writer.render(spec) fn render(&self, spec: &PySpec) -> PyResult { - self.inner - .render(&spec.inner) - .map_err(ggsql_err_to_py) + self.inner.render(&spec.inner).map_err(ggsql_err_to_py) } } @@ -685,8 +698,7 @@ impl PySpec { /// If validation fails unexpectedly (not for syntax errors, which are captured). #[pyfunction] fn validate(query: &str) -> PyResult { - let v = rust_validate(query) - .map_err(ggsql_err_to_py)?; + let v = rust_validate(query).map_err(ggsql_err_to_py)?; Ok(PyValidated { sql: v.sql().to_string(),