kosiew commented on code in PR #1019: URL: https://github.com/apache/datafusion-python/pull/1019#discussion_r2068558333
########## python/tests/test_functions.py: ########## @@ -1236,3 +1236,57 @@ def test_between_default(df): def test_alias_with_metadata(df): df = df.select(f.alias(f.col("a"), "b", {"key": "value"})) assert df.schema().field("b").metadata == {b"key": b"value"} + + +def test_coalesce(df): Review Comment: Added this test because while researching this PR, I initially checked out the coalesce function and found there were no tests yet. ########## src/utils.rs: ########## @@ -87,3 +89,19 @@ pub(crate) fn validate_pycapsule(capsule: &Bound<PyCapsule>, name: &str) -> PyRe Ok(()) } + +pub(crate) fn py_obj_to_scalar_value(py: Python, obj: PyObject) -> PyResult<ScalarValue> { + // convert Python object to PyScalarValue to ScalarValue + + let pa = py.import("pyarrow")?; + + // Convert Python object to PyArrow scalar + let scalar = pa.call_method1("scalar", (obj,))?; + + // Convert PyArrow scalar to PyScalarValue + let py_scalar = PyScalarValue::extract_bound(scalar.as_ref()) + .map_err(|e| PyValueError::new_err(format!("Failed to extract PyScalarValue: {}", e)))?; + + // Convert PyScalarValue to ScalarValue + Ok(py_scalar.into()) +} Review Comment: The above is simpler than the original ```rust fn py_obj_to_scalar_value(py: Python, obj: PyObject) -> ScalarValue { if let Ok(value) = obj.extract::<bool>(py) { ScalarValue::Boolean(Some(value)) } else if let Ok(value) = obj.extract::<i64>(py) { ScalarValue::Int64(Some(value)) } else if let Ok(value) = obj.extract::<u64>(py) { ScalarValue::UInt64(Some(value)) } else if let Ok(value) = obj.extract::<f64>(py) { ScalarValue::Float64(Some(value)) } else if let Ok(value) = obj.extract::<String>(py) { ScalarValue::Utf8(Some(value)) } else { panic!("Unsupported value type") } } ``` which did not handle other python scalars eg datetime ########## src/config.rs: ########## @@ -82,20 +82,3 @@ impl PyConfig { } } } - -/// Convert a python object to a ScalarValue -fn py_obj_to_scalar_value(py: Python, obj: PyObject) -> ScalarValue { - if let Ok(value) = obj.extract::<bool>(py) { - ScalarValue::Boolean(Some(value)) - } else if let Ok(value) = obj.extract::<i64>(py) { - ScalarValue::Int64(Some(value)) - } else if let Ok(value) = obj.extract::<u64>(py) { - ScalarValue::UInt64(Some(value)) - } else if let Ok(value) = obj.extract::<f64>(py) { - ScalarValue::Float64(Some(value)) - } else if let Ok(value) = obj.extract::<String>(py) { - ScalarValue::Utf8(Some(value)) - } else { - panic!("Unsupported value type") - } -} Review Comment: Moved to src/utils.rs with a simpler implementation -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For additional commands, e-mail: github-h...@datafusion.apache.org