[GitHub] [arrow] jorisvandenbossche commented on a diff in pull request #13552: ARROW-18054: [Python][CI] Enable Cython tests on windows wheels

[GitBox <gi...@apache.org>]

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


##########
python/pyarrow/tests/test_cython.py:
##########
@@ -123,18 +123,31 @@ def test_cython_api(tmpdir):
         # pyarrow imported first.
         code = """if 1:
             import sys
+            import os
+
+            try:
+                # Add dll directory was added on python 3.8
+                # and is required in order to find extra DLLs
+                # only for win32
+                for dir in {library_dirs}:
+                    os.add_dll_directory(dir)
+            except AttributeError:
+                pass
 
             mod = __import__({mod_name!r})
             arr = mod.make_null_array(5)
             assert mod.get_array_length(arr) == 5
             assert arr.null_count == 5
-        """.format(mod_name='pyarrow_cython_example')
+        """.format(mod_name='pyarrow_cython_example',
+                   library_dirs=pa.get_library_dirs())
 
-        if sys.platform == 'win32':
+        if sys.platform == 'win32' and not \
+                getattr(os, 'add_dll_directory', False):
+            # Python 3.8 onwards don't check extension module DLLs on path
+            # we have to use os.add_dll_directory instead.

Review Comment:
   This branch is setting `PATH`, so the additional check is to not set the PATH when not needed (it should work without doing that, and we also want to verify that it indeed works without setting it, to ensure we test conda without enabling its DLL path modification patch). 
   
   But yes, if this check returns False, we don't have to set LD_LIBRARY_PATH on Windows, we just have to do nothing. So the `getattr(os, 'add_dll_directory', False)` check can be moved inside the if block as a nested if.



-- 
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-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org