Skip to content

Commit f9943e2

Browse files
georgestagggeorgestagg
committed
Make register a strictly required method
1 parent 943f6fb commit f9943e2

8 files changed

Lines changed: 80 additions & 137 deletions

File tree

‎CLAUDE.md‎

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -994,10 +994,12 @@ spec = ggsql.execute(
994994
)
995995
```
996996

997+
Required methods for custom readers (in addition to `execute_sql`):
998+
999+
- `register(name: str, df: polars.DataFrame, replace: bool = False) -> None` - Register a DataFrame as a table
1000+
9971001
Optional methods for custom readers:
9981002

999-
- `supports_register() -> bool` - Return `True` if registration is supported
1000-
- `register(name: str, df: polars.DataFrame) -> None` - Register a DataFrame as a table
10011003
- `unregister(name: str) -> None` - Unregister a previously registered table
10021004

10031005
Native readers (e.g., `DuckDBReader`) use an optimized fast path, while custom Python readers are automatically bridged via IPC serialization.
@@ -1091,15 +1093,15 @@ Where `<global_mapping>` can be:
10911093

10921094
### Clause Types
10931095

1094-
| Clause | Repeatable | Purpose | Example |
1095-
| -------------- | ---------- | ------------------ | ------------------------------------ |
1096-
| `VISUALISE` | ✅ Yes | Entry point | `VISUALISE date AS x, revenue AS y` |
1097-
| `DRAW` | ✅ Yes | Define layers | `DRAW line MAPPING date AS x, value AS y` |
1098-
| `SCALE` | ✅ Yes | Configure scales | `SCALE x VIA date` |
1099-
| `FACET` | ❌ No | Small multiples | `FACET WRAP region` |
1100-
| `COORD` | ❌ No | Coordinate system | `COORD cartesian SETTING xlim => [0,100]` |
1101-
| `LABEL` | ❌ No | Text labels | `LABEL title => 'My Chart', x => 'Date'` |
1102-
| `THEME` | ❌ No | Visual styling | `THEME minimal` |
1096+
| Clause | Repeatable | Purpose | Example |
1097+
| ----------- | ---------- | ----------------- | ----------------------------------------- |
1098+
| `VISUALISE` | ✅ Yes | Entry point | `VISUALISE date AS x, revenue AS y` |
1099+
| `DRAW` | ✅ Yes | Define layers | `DRAW line MAPPING date AS x, value AS y` |
1100+
| `SCALE` | ✅ Yes | Configure scales | `SCALE x VIA date` |
1101+
| `FACET` | ❌ No | Small multiples | `FACET WRAP region` |
1102+
| `COORD` | ❌ No | Coordinate system | `COORD cartesian SETTING xlim => [0,100]` |
1103+
| `LABEL` | ❌ No | Text labels | `LABEL title => 'My Chart', x => 'Date'` |
1104+
| `THEME` | ❌ No | Visual styling | `THEME minimal` |
11031105

11041106
### DRAW Clause (Layers)
11051107

‎ggsql-python/README.md‎

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -125,10 +125,9 @@ reader = ggsql.DuckDBReader("duckdb:///path/to/file.db") # File database
125125

126126
**Methods:**
127127

128-
- `register(name: str, df: polars.DataFrame)` - Register a DataFrame as a queryable table
128+
- `register(name: str, df: polars.DataFrame, replace: bool = False)` - Register a DataFrame as a queryable table
129129
- `unregister(name: str)` - Unregister a previously registered table
130130
- `execute_sql(sql: str) -> polars.DataFrame` - Execute SQL and return results
131-
- `supports_register() -> bool` - Check if registration is supported
132131

133132
#### `VegaLiteWriter()`
134133

@@ -262,11 +261,10 @@ writer = ggsql.VegaLiteWriter()
262261
json_output = writer.render(spec)
263262
```
264263

265-
**Optional methods** for custom readers:
264+
**Additional methods** for custom readers:
266265

267-
- `supports_register() -> bool` - Return `True` if your reader supports DataFrame registration
268-
- `register(name: str, df: polars.DataFrame) -> None` - Register a DataFrame as a queryable table
269-
- `unregister(name: str) -> None` - Unregister a previously registered table
266+
- `register(name: str, df: polars.DataFrame, replace: bool = False) -> None` - Register a DataFrame as a queryable table (required)
267+
- `unregister(name: str) -> None` - Unregister a previously registered table (optional)
270268

271269
```python
272270
class AdvancedReader:
@@ -279,10 +277,7 @@ class AdvancedReader:
279277
# Your SQL execution logic here
280278
...
281279

282-
def supports_register(self) -> bool:
283-
return True
284-
285-
def register(self, name: str, df: pl.DataFrame) -> None:
280+
def register(self, name: str, df: pl.DataFrame, replace: bool = False) -> None:
286281
self.tables[name] = df
287282

288283
def unregister(self, name: str) -> None:
@@ -313,11 +308,8 @@ class IbisReader:
313308
def execute_sql(self, sql: str) -> pl.DataFrame:
314309
return self.con.con.execute(sql).pl()
315310

316-
def supports_register(self) -> bool:
317-
return True
318-
319-
def register(self, name: str, df: pl.DataFrame) -> None:
320-
self.con.create_table(name, df.to_arrow(), overwrite=True)
311+
def register(self, name: str, df: pl.DataFrame, replace: bool = False) -> None:
312+
self.con.create_table(name, df.to_arrow(), overwrite=replace)
321313

322314
def unregister(self, name: str) -> None:
323315
self.con.drop_table(name)

‎ggsql-python/src/lib.rs‎

Lines changed: 1 addition & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -138,16 +138,6 @@ impl Reader for PyReaderBridge {
138138
})
139139
}
140140

141-
fn supports_register(&self) -> bool {
142-
Python::attach(|py| {
143-
self.obj
144-
.bind(py)
145-
.call_method0("supports_register")
146-
.and_then(|r| r.extract::<bool>())
147-
.unwrap_or(false)
148-
})
149-
}
150-
151141
fn register(&self, name: &str, df: DataFrame, replace: bool) -> ggsql::Result<()> {
152142
Python::attach(|py| {
153143
let py_df =
@@ -254,6 +244,7 @@ impl PyDuckDBReader {
254244
/// ------
255245
/// ValueError
256246
/// If registration fails or the table name is invalid.
247+
#[pyo3(signature = (name, df, replace=false))]
257248
fn register(
258249
&self,
259250
py: Python<'_>,
@@ -308,16 +299,6 @@ impl PyDuckDBReader {
308299
polars_to_py(py, &df)
309300
}
310301

311-
/// Check if this reader supports DataFrame registration.
312-
///
313-
/// Returns
314-
/// -------
315-
/// bool
316-
/// True if register() is supported, False otherwise.
317-
fn supports_register(&self) -> bool {
318-
self.inner.supports_register()
319-
}
320-
321302
/// Execute a ggsql query and return the visualization specification.
322303
///
323304
/// This is the main entry point for creating visualizations. It parses

‎ggsql-python/tests/test_ggsql.py‎

Lines changed: 52 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -84,10 +84,6 @@ def test_register_and_query(self):
8484
assert isinstance(result, pl.DataFrame)
8585
assert result.shape == (2, 2)
8686

87-
def test_supports_register(self):
88-
reader = ggsql.DuckDBReader("duckdb://memory")
89-
assert reader.supports_register() is True
90-
9187
def test_invalid_connection_string(self):
9288
with pytest.raises(ValueError):
9389
ggsql.DuckDBReader("invalid://connection")
@@ -396,38 +392,24 @@ class TestCustomReader:
396392
"""Tests for custom Python reader support."""
397393

398394
def test_simple_custom_reader(self):
399-
"""Custom reader with execute_sql() method works."""
395+
"""Custom reader works."""
400396

401397
class SimpleReader:
402-
def execute_sql(self, sql: str) -> pl.DataFrame:
403-
return pl.DataFrame({"x": [1, 2, 3], "y": [10, 20, 30]})
404-
405-
reader = SimpleReader()
406-
spec = ggsql.execute("SELECT * FROM data VISUALISE x, y DRAW point", reader)
407-
assert spec.metadata()["rows"] == 3
408-
409-
def test_custom_reader_with_register(self):
410-
"""Custom reader with register() support."""
411-
412-
class RegisterReader:
413398
def __init__(self):
414-
self.tables = {}
399+
self.ctx = pl.SQLContext()
415400

416401
def execute_sql(self, sql: str) -> pl.DataFrame:
417-
# Simple: just return the first registered table
418-
if self.tables:
419-
return next(iter(self.tables.values()))
420-
return pl.DataFrame({"x": [1], "y": [2]})
421-
422-
def supports_register(self) -> bool:
423-
return True
402+
return self.ctx.execute(sql).collect()
424403

425-
def register(self, name: str, df: pl.DataFrame) -> None:
426-
self.tables[name] = df
404+
def register(
405+
self, name: str, df: pl.DataFrame, replace: bool = False
406+
) -> None:
407+
self.ctx.register(name, df)
427408

428-
reader = RegisterReader()
429-
spec = ggsql.execute("SELECT 1 AS x, 2 AS y VISUALISE x, y DRAW point", reader)
430-
assert spec is not None
409+
reader = SimpleReader()
410+
reader.register("data", pl.DataFrame({"x": [1, 2, 3], "y": [10, 20, 30]}))
411+
spec = ggsql.execute("SELECT * FROM data VISUALISE x, y DRAW point", reader)
412+
assert spec.metadata()["rows"] == 3
431413

432414
def test_custom_reader_error_handling(self):
433415
"""Custom reader errors are propagated."""
@@ -436,6 +418,11 @@ class ErrorReader:
436418
def execute_sql(self, sql: str) -> pl.DataFrame:
437419
raise ValueError("Custom reader error")
438420

421+
def register(
422+
self, name: str, df: pl.DataFrame, replace: bool = False
423+
) -> None:
424+
raise ValueError("Custom reader error")
425+
439426
reader = ErrorReader()
440427
with pytest.raises(ValueError, match="Custom reader error"):
441428
ggsql.execute("SELECT 1 VISUALISE x, y DRAW point", reader)
@@ -444,9 +431,17 @@ def test_custom_reader_wrong_return_type(self):
444431
"""Custom reader returning wrong type raises TypeError."""
445432

446433
class WrongTypeReader:
434+
def __init__(self):
435+
self.ctx = pl.SQLContext()
436+
447437
def execute_sql(self, sql: str):
448438
return {"x": [1, 2, 3]} # dict, not DataFrame
449439

440+
def register(
441+
self, name: str, df: pl.DataFrame, replace: bool = False
442+
) -> None:
443+
self.ctx.register(name, df)
444+
450445
reader = WrongTypeReader()
451446
with pytest.raises((ValueError, TypeError)):
452447
ggsql.execute("SELECT 1 VISUALISE x, y DRAW point", reader)
@@ -461,16 +456,28 @@ def test_custom_reader_can_render(self):
461456
"""Custom reader result can be rendered to Vega-Lite."""
462457

463458
class StaticReader:
459+
def __init__(self):
460+
self.ctx = pl.SQLContext()
461+
464462
def execute_sql(self, sql: str) -> pl.DataFrame:
465-
return pl.DataFrame(
466-
{
467-
"x": [1, 2, 3, 4, 5],
468-
"y": [10, 40, 20, 50, 30],
469-
"category": ["A", "B", "A", "B", "A"],
470-
}
471-
)
463+
return self.ctx.execute(sql).collect()
464+
465+
def register(
466+
self, name: str, df: pl.DataFrame, replace: bool = False
467+
) -> None:
468+
self.ctx.register(name, df)
472469

473470
reader = StaticReader()
471+
reader.register(
472+
"data",
473+
pl.DataFrame(
474+
{
475+
"x": [1, 2, 3, 4, 5],
476+
"y": [10, 40, 20, 50, 30],
477+
"category": ["A", "B", "A", "B", "A"],
478+
}
479+
),
480+
)
474481
spec = ggsql.execute(
475482
"SELECT * FROM data VISUALISE x, y, category AS color DRAW point",
476483
reader,
@@ -488,13 +495,20 @@ def test_custom_reader_execute_sql_called(self):
488495

489496
class RecordingReader:
490497
def __init__(self):
498+
self.ctx = pl.SQLContext()
491499
self.execute_calls = []
492500

493501
def execute_sql(self, sql: str) -> pl.DataFrame:
494502
self.execute_calls.append(sql)
495-
return pl.DataFrame({"x": [1], "y": [2]})
503+
return self.ctx.execute(sql).collect()
504+
505+
def register(
506+
self, name: str, df: pl.DataFrame, replace: bool = False
507+
) -> None:
508+
self.ctx.register(name, df)
496509

497510
reader = RecordingReader()
511+
reader.register("data", pl.DataFrame({"x": [1], "y": [2]}))
498512
ggsql.execute(
499513
"SELECT * FROM data VISUALISE x, y DRAW point",
500514
reader,
@@ -516,10 +530,7 @@ def __init__(self):
516530
def execute_sql(self, sql: str) -> pl.DataFrame:
517531
return self.con.con.execute(sql).pl()
518532

519-
def supports_register(self) -> bool:
520-
return True
521-
522-
def register(self, name: str, df: pl.DataFrame) -> None:
533+
def register(self, name: str, df: pl.DataFrame, replace: bool = False) -> None:
523534
self.con.create_table(name, df.to_arrow(), overwrite=True)
524535

525536
def unregister(self, name: str) -> None:

‎src/doc/API.md‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -374,13 +374,10 @@ pub trait Reader {
374374
fn execute_sql(&self, sql: &str) -> Result<DataFrame>;
375375

376376
/// Register a DataFrame as a queryable table
377-
fn register(&mut self, name: &str, df: DataFrame) -> Result<()>;
377+
fn register(&self, name: &str, df: DataFrame, replace: bool) -> Result<()>;
378378

379379
/// Unregister a previously registered table
380-
fn unregister(&mut self, name: &str) -> Result<()>;
381-
382-
/// Check if this reader supports DataFrame registration
383-
fn supports_register(&self) -> bool;
380+
fn unregister(&self, name: &str) -> Result<()>;
384381
}
385382
```
386383

@@ -434,9 +431,6 @@ class DuckDBReader:
434431

435432
def execute_sql(self, sql: str) -> polars.DataFrame:
436433
"""Execute SQL and return a Polars DataFrame."""
437-
438-
def supports_register(self) -> bool:
439-
"""Check if registration is supported."""
440434
```
441435

442436
#### `VegaLiteWriter`

‎src/reader/duckdb.rs‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -555,9 +555,6 @@ impl Reader for DuckDBReader {
555555
Ok(())
556556
}
557557

558-
fn supports_register(&self) -> bool {
559-
true
560-
}
561558
}
562559

563560
#[cfg(test)]
@@ -709,12 +706,6 @@ mod tests {
709706
.contains("exceeds maximum length"));
710707
}
711708

712-
#[test]
713-
fn test_supports_register() {
714-
let reader = DuckDBReader::from_connection_string("duckdb://memory").unwrap();
715-
assert!(reader.supports_register());
716-
}
717-
718709
#[test]
719710
fn test_register_empty_dataframe() {
720711
let reader = DuckDBReader::from_connection_string("duckdb://memory").unwrap();

‎src/reader/mod.rs‎

Lines changed: 4 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ pub struct Metadata {
9797
///
9898
/// # DataFrame Registration
9999
///
100-
/// Some readers support registering DataFrames as queryable tables using
100+
/// Readers support registering DataFrames as queryable tables using
101101
/// the [`register`](Reader::register) method. This allows you to query
102102
/// in-memory DataFrames with SQL, join them with other tables, etc.
103103
///
@@ -143,17 +143,8 @@ pub trait Reader {
143143
///
144144
/// # Returns
145145
///
146-
/// `Ok(())` on success, error if registration fails or isn't supported.
147-
///
148-
/// # Default Implementation
149-
///
150-
/// Returns an error by default. Override for readers that support registration.
151-
fn register(&self, name: &str, _df: DataFrame, _replace: bool) -> Result<()> {
152-
Err(GgsqlError::ReaderError(format!(
153-
"This reader does not support DataFrame registration for table '{}'",
154-
name
155-
)))
156-
}
146+
/// `Ok(())` on success, error if registration fails.
147+
fn register(&self, name: &str, df: DataFrame, replace: bool) -> Result<()>;
157148

158149
/// Unregister a previously registered table
159150
///
@@ -167,23 +158,14 @@ pub trait Reader {
167158
///
168159
/// # Default Implementation
169160
///
170-
/// Returns an error by default. Override for readers that support registration.
161+
/// Returns an error by default. Override for readers that support unregistration.
171162
fn unregister(&self, name: &str) -> Result<()> {
172163
Err(GgsqlError::ReaderError(format!(
173164
"This reader does not support unregistering table '{}'",
174165
name
175166
)))
176167
}
177168

178-
/// Check if this reader supports DataFrame registration
179-
///
180-
/// # Returns
181-
///
182-
/// `true` if [`register`](Reader::register) is implemented, `false` otherwise.
183-
fn supports_register(&self) -> bool {
184-
false
185-
}
186-
187169
/// Execute a ggsql query and return the visualization specification.
188170
///
189171
/// This is the main entry point for creating visualizations. It parses the query,

0 commit comments

Comments
 (0)