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

Reply via email to