Skip to content

Commit 274fc3a

Browse files
timsaucerclaude
andcommitted
Address PR review feedback for SessionContext utility methods
- Improve docstring examples to show actual output instead of asserts - Use doctest +SKIP for non-deterministic session_start_time output - Fix table_provider error mapping: outer async error is now RuntimeError - Strengthen tests: validate RFC 3339 with fromisoformat, test both optimizer rule removal paths, exact string match for parse_sql_expr, verify enable_ident_normalization with dynamic state change Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent c7f47f1 commit 274fc3a

3 files changed

Lines changed: 19 additions & 11 deletions

File tree

crates/core/src/context.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ use datafusion_python_util::{
6060
};
6161
use object_store::ObjectStore;
6262
use pyo3::IntoPyObjectExt;
63-
use pyo3::exceptions::{PyKeyError, PyValueError};
63+
use pyo3::exceptions::{PyKeyError, PyRuntimeError, PyValueError};
6464
use pyo3::prelude::*;
6565
use pyo3::types::{PyCapsule, PyDict, PyList, PyTuple};
6666
use url::Url;
@@ -1088,7 +1088,9 @@ impl PySessionContext {
10881088

10891089
pub fn table_provider(&self, name: &str, py: Python) -> PyResult<PyTable> {
10901090
let provider = wait_for_future(py, self.ctx.table_provider(name))
1091-
.map_err(|e| PyKeyError::new_err(e.to_string()))?
1091+
// Outer error: runtime/async failure
1092+
.map_err(|e| PyRuntimeError::new_err(e.to_string()))?
1093+
// Inner error: table not found
10921094
.map_err(|e| PyKeyError::new_err(e.to_string()))?;
10931095
Ok(PyTable { table: provider })
10941096
}

python/datafusion/context.py

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1147,8 +1147,8 @@ def session_start_time(self) -> str:
11471147
11481148
Examples:
11491149
>>> ctx = SessionContext()
1150-
>>> start_time = ctx.session_start_time()
1151-
>>> assert "T" in start_time # RFC 3339 contains a 'T' separator
1150+
>>> ctx.session_start_time() # doctest: +SKIP
1151+
'2026-01-01T12:34:56.123456789+00:00'
11521152
"""
11531153
return self.ctx.session_start_time()
11541154

@@ -1157,7 +1157,8 @@ def enable_ident_normalization(self) -> bool:
11571157
11581158
Examples:
11591159
>>> ctx = SessionContext()
1160-
>>> assert isinstance(ctx.enable_ident_normalization(), bool)
1160+
>>> ctx.enable_ident_normalization()
1161+
True
11611162
"""
11621163
return self.ctx.enable_ident_normalization()
11631164

@@ -1175,8 +1176,8 @@ def parse_sql_expr(self, sql: str, schema: DFSchema) -> Expr:
11751176
>>> from datafusion.common import DFSchema
11761177
>>> ctx = SessionContext()
11771178
>>> schema = DFSchema.empty()
1178-
>>> expr = ctx.parse_sql_expr("1 + 2", schema)
1179-
>>> assert "Int64(1) + Int64(2)" in str(expr)
1179+
>>> ctx.parse_sql_expr("1 + 2", schema=schema)
1180+
Expr(Int64(1) + Int64(2))
11801181
"""
11811182
from datafusion.expr import Expr # noqa: PLC0415
11821183

python/tests/test_context.py

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -552,22 +552,26 @@ def test_table_not_found(ctx):
552552

553553

554554
def test_session_start_time(ctx):
555+
import datetime
556+
555557
st = ctx.session_start_time()
556558
assert isinstance(st, str)
557-
assert "T" in st # RFC 3339 format
559+
dt = datetime.datetime.fromisoformat(st)
560+
assert dt.isoformat()
558561

559562

560563
def test_enable_ident_normalization(ctx):
561-
result = ctx.enable_ident_normalization()
562-
assert isinstance(result, bool)
564+
assert ctx.enable_ident_normalization() is True
565+
ctx.sql("SET datafusion.sql_parser.enable_ident_normalization = false")
566+
assert ctx.enable_ident_normalization() is False
563567

564568

565569
def test_parse_sql_expr(ctx):
566570
from datafusion.common import DFSchema
567571

568572
schema = DFSchema.empty()
569573
expr = ctx.parse_sql_expr("1 + 2", schema)
570-
assert "Int64(1) + Int64(2)" in str(expr)
574+
assert str(expr) == "Expr(Int64(1) + Int64(2))"
571575

572576

573577
def test_execute_logical_plan(ctx):
@@ -583,6 +587,7 @@ def test_refresh_catalogs(ctx):
583587

584588

585589
def test_remove_optimizer_rule(ctx):
590+
assert ctx.remove_optimizer_rule("push_down_filter") is True
586591
assert ctx.remove_optimizer_rule("nonexistent_rule") is False
587592

588593

0 commit comments

Comments
 (0)