pitrou commented on code in PR #13311: URL: https://github.com/apache/arrow/pull/13311#discussion_r949065096
########## ci/scripts/python_test.sh: ########## @@ -54,4 +55,14 @@ export PYARROW_TEST_ORC export PYARROW_TEST_PARQUET export PYARROW_TEST_S3 +# Testing PyArrow CPP Review Comment: ```suggestion # Testing PyArrow C++ ``` ########## docs/source/developers/python.rst: ########## @@ -131,6 +131,30 @@ for ``.py`` files or for ``.pyx`` and ``.pxi`` files. In this case you will also need to install the `pytest-cython <https://github.com/lgpage/pytest-cython>`_ plugin. +Testing PyArrow CPP Review Comment: ```suggestion Testing PyArrow C++ ``` ########## docs/source/developers/python.rst: ########## @@ -131,6 +131,30 @@ for ``.py`` files or for ``.pyx`` and ``.pxi`` files. In this case you will also need to install the `pytest-cython <https://github.com/lgpage/pytest-cython>`_ plugin. +Testing PyArrow CPP +------------------- + +If you want to run ctest for the tests that are included in the PyArrow CPP +module, you will need to build Arrow with ``-DARROW_BUILD_TESTS=ON``. + +.. note:: + + Currently building the C++ unit tests does not work with googletest + from conda-forge, so we must use the ``BUNDLED`` source for building that + dependency + + In case you use conda add ``-DGTest_SOURCE=BUNDLED`` to the CMake flags + when building Arrow. Review Comment: ```suggestion Currently, building the PyArrow C++ unit tests does not work with the googletest package from conda-forge. If you are in this situation, please add ``-DGTest_SOURCE=BUNDLED`` to the CMake flags when building Arrow C++. ``` ########## docs/source/developers/python.rst: ########## @@ -131,6 +131,30 @@ for ``.py`` files or for ``.pyx`` and ``.pxi`` files. In this case you will also need to install the `pytest-cython <https://github.com/lgpage/pytest-cython>`_ plugin. +Testing PyArrow CPP +------------------- + +If you want to run ctest for the tests that are included in the PyArrow CPP +module, you will need to build Arrow with ``-DARROW_BUILD_TESTS=ON``. Review Comment: ```suggestion Most of the tests for PyArrow are part of the ``pytest``-based test suite mentioned above, but a few low-level tests are written directly in C++ for historical reasons. Those tests can be run using ``ctest``, but you first will need to build Arrow C++ with ``-DARROW_BUILD_TESTS=ON``. ``` ########## docs/source/developers/python.rst: ########## @@ -131,6 +131,30 @@ for ``.py`` files or for ``.pyx`` and ``.pxi`` files. In this case you will also need to install the `pytest-cython <https://github.com/lgpage/pytest-cython>`_ plugin. +Testing PyArrow CPP +------------------- + +If you want to run ctest for the tests that are included in the PyArrow CPP +module, you will need to build Arrow with ``-DARROW_BUILD_TESTS=ON``. + +.. note:: + + Currently building the C++ unit tests does not work with googletest + from conda-forge, so we must use the ``BUNDLED`` source for building that + dependency + + In case you use conda add ``-DGTest_SOURCE=BUNDLED`` to the CMake flags + when building Arrow. + +After Arrow C++ and PyArrow are built, navigate to ``python/build/dist`` +folder and run ctest: Review Comment: ```suggestion After Arrow C++ and PyArrow are built, you can navigate to the ``python/build/dist`` folder and run ``ctest``: ``` ########## docs/source/developers/python.rst: ########## @@ -391,6 +415,12 @@ variable to 1. To set the number of threads used to compile PyArrow's C++/Cython components, set the ``PYARROW_PARALLEL`` environment variable. +.. note:: + + If you used a different directory name for building Arrow C++ (by default it is + named "build"), then also set `ARROW_BUILD_DIR='name_of_build_dir'`. This way + pyarrow can find Arrow C++ built files. Review Comment: ```suggestion If you used a different directory name for building Arrow C++ (by default it is named "build"), then you should also set the environment variable ``ARROW_BUILD_DIR='name_of_build_dir'``. This way PyArrow can find the Arrow C++ built files. ``` ########## python/setup.py: ########## @@ -227,6 +228,126 @@ def initialize_options(self): '_hdfsio', 'gandiva'] + def _run_cmake_pyarrow_cpp(self): + # check if build_type is correctly passed / set + if self.build_type.lower() not in ('release', 'debug'): Review Comment: For the record, can we open a JIRA to also support `RelWithDebInfo`? ########## python/pyarrow/tests/test_cython.py: ########## @@ -136,9 +138,9 @@ def test_cython_api(tmpdir): delim, var = ':', 'LD_LIBRARY_PATH' subprocess_env[var] = delim.join( - pa.get_library_dirs() + [subprocess_env.get(var, '')] + pa.get_library_dirs() + + [subprocess_env.get(var, '')] + [str(tmpdir)] Review Comment: I'm curious, why was it necessary to add `tmpdir` here? Ideally, `pa.get_library_dirs()` is sufficient. -- 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]
