jorisvandenbossche commented on code in PR #41904:
URL: https://github.com/apache/arrow/pull/41904#discussion_r1629152457


##########
python/pyarrow/_compute.pyx:
##########
@@ -473,7 +476,7 @@ cdef class MetaFunction(Function):
 
 cdef _pack_compute_args(object values, vector[CDatum]* out):
     for val in values:
-        if isinstance(val, (list, np.ndarray)):
+        if "numpy" in sys.modules and isinstance(val, (list, np.ndarray)):

Review Comment:
   For on the Python/cython side, I think we can have a simple globally used 
variable like `HAS_NUMPY` that is populated once (eg in lib.pyx), to avoid 
using the `"numpy" in sys.modules` pattern every time for checking if numpy is 
available



##########
python/pyarrow/tests/test_adhoc_memory_leak.py:
##########
@@ -28,6 +31,7 @@
     pass
 
 
[email protected]

Review Comment:
   if something is already labeled as requiring pandas, it shouldn't need to be 
labeled as also requiring numpy?



##########
python/pyarrow/builder.pxi:
##########
@@ -42,7 +42,7 @@ cdef class StringBuilder(_Weakrefable):
         value : string/bytes or np.nan/None
             The value to append to the string array builder.
         """
-        if value is None or value is np.nan:
+        if value is None or ('numpy' in sys.modules and value is np.nan):

Review Comment:
   Not directly related to this PR (and I assume this code is not used much), 
but it's also a bit strange that this checks explicitly for np.nan in addition 
to None (it could also check for `float("nan")`, for example), and additionally 
checking for NaN with `is np.nan` is also wrong because that doesn't always 
give true .. (eg `np.array([np.nan])[0] is np.nan` is False)



##########
python/pyarrow/src/arrow/python/init.cc:
##########
@@ -21,4 +21,15 @@
 #include "arrow/python/init.h"
 #include "arrow/python/numpy_interop.h"
 
-int arrow_init_numpy() { return arrow::py::import_numpy(); }
+bool numpy_imported = false;
+
+int arrow_init_numpy(bool import_numpy) {
+  if (import_numpy) {
+    numpy_imported = true;
+    return arrow::py::import_numpy();
+  } else {
+    return 0;
+  }
+}
+
+bool get_numpy_imported() { return numpy_imported; }

Review Comment:
   `is_numpy_imported`? (or `has_numpy` ?)



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to