pitrou commented on a change in pull request #11505:
URL: https://github.com/apache/arrow/pull/11505#discussion_r744841733



##########
File path: docs/source/developers/python.rst
##########
@@ -344,8 +349,11 @@ Now, build pyarrow:
    python setup.py build_ext --inplace
    popd
 
-If you did not build one of the optional components, set the corresponding
-``PYARROW_WITH_$COMPONENT`` environment variable to 0.
+If you did build one of the optional components (in C++), you need to set the
+corresponding ``PYARROW_WITH_$COMPONENT`` environment variable to 1.
+
+If you wish to delete pyarrow build before rebuilding navigate to the 
``arrow/python``
+folder and run ``python setup.py clean``.

Review comment:
       `git clean -Xfd .` is probably more thorough.

##########
File path: docs/source/developers/python.rst
##########
@@ -55,12 +55,15 @@ like so:
 
 .. code-block:: shell
 
-   pytest pyarrow
+   pytest arrow/python/pyarrow

Review comment:
       This really depends on which directory your shell is currently in. If 
you just ran something like `pip install -e .`, then presumably you're already 
inside the `python` directory.
   
   Also, I would expect Python programmers to understand how to get this right.

##########
File path: docs/source/developers/python.rst
##########
@@ -106,8 +109,8 @@ Building on Linux and MacOS
 System Requirements
 -------------------
 
-On macOS, any modern XCode (6.4 or higher; the current version is 10) is
-sufficient.
+On macOS, any modern XCode (6.4 or higher; the current version is 13) is
+sufficient. All you need is to have XCode installed, no additional 
configuration is needed.

Review comment:
       Is this useful? The preceding sentence already says that "XCode is 
sufficient".

##########
File path: docs/source/developers/python.rst
##########
@@ -55,12 +55,15 @@ like so:
 
 .. code-block:: shell
 
-   pytest pyarrow
+   pytest arrow/python/pyarrow
 
 Package requirements to run the unit tests are found in
 ``requirements-test.txt`` and can be installed if needed with ``pip install -r
 requirements-test.txt``.
 
+If the test fails to connect to libarrow run ``python -m pytest pyarrow`` and
+check if the editable version of pyarrow was installed correctly.

Review comment:
       "connect to" isn't really a faithful representation of what happens 
here. Perhaps:
   
   """If you get import errors for `pyarrow._lib` or another PyArrow module 
when trying to run the tests, check if..."""

##########
File path: docs/source/developers/python.rst
##########
@@ -306,6 +302,10 @@ adding flags with ``ON``:
 Anything set to ``ON`` above can also be turned off. Note that some compression
 libraries are needed for Parquet support.
 
+For more debugging information add flag
+``-DCMAKE_BUILD_TYPE=debug``. Read more about build type in C++ building 
section
+:ref:`cpp-building-building`.

Review comment:
       I would say:
   
   ```rest
   To enable C++ debugging information, pass the option 
``-DCMAKE_BUILD_TYPE=debug``.
   
   .. seealso::
      :ref:`cpp-building-building`.
   ```

##########
File path: docs/source/developers/python.rst
##########
@@ -359,6 +367,10 @@ libraries), one can set ``--bundle-arrow-cpp``:
    python setup.py build_ext --build-type=$ARROW_BUILD_TYPE \
           --bundle-arrow-cpp bdist_wheel
 
+.. note::
+   To run an editable pyarrow version run ``pip install -e . 
--no-build-isolation``

Review comment:
       I would say: "To install an editable PyArrow build ...".

##########
File path: docs/source/developers/python.rst
##########
@@ -248,8 +243,8 @@ folder as the repositories and a target installation folder:
 
 .. code-block:: shell
 
-   virtualenv pyarrow
-   source ./pyarrow/bin/activate
+   virtualenv -p python3.9 pyarrow-dev

Review comment:
       I didn't know this flag existed.
   For the record, it is sufficient these days to use the built-in 
[venv](https://docs.python.org/3/library/venv.html) module, while `virtualenv` 
needs to be installed separately. That said, I suppose many developers are more 
used to `virtualenv`.

##########
File path: docs/source/developers/python.rst
##########
@@ -335,6 +335,11 @@ virtualenv) enables cmake to choose the python executable 
which you are using.
 
 For any other C++ build challenges, see :ref:`cpp-development`.
 
+In case you may need to rebuild the C++ part due to errors in the process it is
+advisable to delete the content of the build folder with command ``rm -rf *``
+from the build folder itself. If the build has passed successfully and you 
need to rebuild

Review comment:
       I would advocate `git clean -Xfd .` (which removes any git-ignored file) 
rather than `rm -rf *`. You don't have to run it from the exact directory.




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