You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/10/11 07:28:58 UTC

[GitHub] [arrow] jorisvandenbossche commented on a diff in pull request #14350: ARROW-17891: [Docs][Python] Update and sync Win section of the developers/python page

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


##########
docs/source/developers/python.rst:
##########
@@ -518,13 +518,11 @@ We set a number of environment variables:
 
 - the path of the installation directory of the Arrow C++ libraries as
   ``ARROW_HOME``
-- add the path of installed DLL libraries to ``PATH``
 - and the CMake generator to be used as ``PYARROW_CMAKE_GENERATOR``
 
 .. code-block::
 
-   $ set ARROW_HOME=%cd%\arrow-dist
-   $ set PATH=%ARROW_HOME%\bin;%PATH%
+   $ set ARROW_HOME=%CONDA_PREFIX%

Review Comment:
   The Appveyor build script is using `%CONDA_PREFIX%\Library`, so it would be good to verify if that "Library" part is needed.
   
   https://github.com/apache/arrow/blob/a1d7a449295adcc1f407d7b0037fbc308492372a/ci/appveyor-cpp-build.bat#L123



##########
docs/source/developers/python.rst:
##########
@@ -518,13 +518,11 @@ We set a number of environment variables:
 
 - the path of the installation directory of the Arrow C++ libraries as
   ``ARROW_HOME``
-- add the path of installed DLL libraries to ``PATH``
 - and the CMake generator to be used as ``PYARROW_CMAKE_GENERATOR``
 
 .. code-block::
 
-   $ set ARROW_HOME=%cd%\arrow-dist
-   $ set PATH=%ARROW_HOME%\bin;%PATH%
+   $ set ARROW_HOME=%CONDA_PREFIX%
    $ set PYARROW_CMAKE_GENERATOR=Visual Studio 15 2017 Win64

Review Comment:
   This could also be "Ninja"? (based on the appveyor script) 
   
   But I am not familiar enough with Windows to know if there is a good reason to prefer one or the other (on Linux we recommend to use Ninja)



##########
docs/source/developers/python.rst:
##########
@@ -576,14 +574,6 @@ Then run the unit tests with:
 
 .. note::
 
-   With the above instructions the Arrow C++ libraries are not bundled with
-   the Python extension. This is recommended for development as it allows the
-   C++ libraries to be re-built separately.

Review Comment:
   This first paragraph can be kept?



##########
docs/source/developers/python.rst:
##########
@@ -509,7 +509,7 @@ First, starting from a fresh clone of Apache Arrow:
          --file arrow\ci\conda_env_cpp.txt ^

Review Comment:
   Here, we don't include `conda_env_unix.txt` since that are Linux/MacOS specific packages. 
   But, I see that that file includes `ccache` and there are windows conda packages for this. Should we move that package to the `conda_env_cpp.txt`? (or otherwise list here manually)



##########
docs/source/developers/python.rst:
##########
@@ -518,13 +518,11 @@ We set a number of environment variables:
 
 - the path of the installation directory of the Arrow C++ libraries as
   ``ARROW_HOME``

Review Comment:
   I would add something like "(when using a conda environment, Arrow C++ is installed in the env directories)"
   
   (to explain why we are setting it to `%CONDA_PREFIX` below)



-- 
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