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 2021/10/21 13:17:01 UTC
[GitHub] [arrow] AlenkaF opened a new pull request #11505: MINOR: [Docs][Python] Proposed changes for building pyarrow docs
AlenkaF opened a new pull request #11505:
URL: https://github.com/apache/arrow/pull/11505
I added some additional information and some changes to the documentation for building pyarrow from source. The changes are meant as a proposal to make this part of the documentation more clear and up-to-date and not as a final correction.
--
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
[GitHub] [arrow] pitrou commented on a change in pull request #11505: MINOR: [Docs][Python] Proposed changes for building pyarrow docs
Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #11505:
URL: https://github.com/apache/arrow/pull/11505#discussion_r745506153
##########
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:
Ah, agreed. Yes, in this case the `rm -rf <build dir>` is more explicit (also, for advanced uses you may have several build directories).
--
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
[GitHub] [arrow] jorisvandenbossche commented on a change in pull request #11505: MINOR: [Docs][Python] Proposed changes for building pyarrow docs
Posted by GitBox <gi...@apache.org>.
jorisvandenbossche commented on a change in pull request #11505:
URL: https://github.com/apache/arrow/pull/11505#discussion_r733775843
##########
File path: docs/source/developers/python.rst
##########
@@ -57,6 +57,8 @@ like so:
pytest pyarrow
+Be sure to run the command inside arrow/python folder.
Review comment:
Another option could be to change the command above to `pytest python/pyarrow`, assuming you are still in the root of the repo?
(personally I always do that, instead of moving to the python directory, but that's maybe only my personal habit ..)
##########
File path: docs/source/developers/cpp/building.rst
##########
@@ -121,9 +121,23 @@ argument is omitted then a release build will be produced.
.. note::
- You need to more options to build on Windows. See
+ You need to set more options to build on Windows. See
:ref:`developers-cpp-windows` for details.
+Other options for build type are also possible:
+
+* ``Debug``: doesn't apply any optimization passes in the compiler and
+ leaves the symbol map in the binary.
+* ``FastDebug``
+* ``RelWithDebInfo``: applies compiler optimizations and it removes the symbol
+ map from the binary.
+* ``Release``: fast but less useful for debugging or profiling.
+* ``Profile_Gen``
+* ``Profile_Build``
+
+You can also run default build with flag `-DARROW_EXTRA_ERROR_CONTEXT=ON`, see
+:ref:`Extra debugging help`
Review comment:
For referencing another section in the document, you will need to add a "label" to that section (if one doesn't exist yet) and use that label here instead of the title. See https://www.sphinx-doc.org/en/master/usage/restructuredtext/roles.html#ref-role for some explanation of that.
##########
File path: docs/source/developers/cpp/building.rst
##########
@@ -121,9 +121,23 @@ argument is omitted then a release build will be produced.
.. note::
- You need to more options to build on Windows. See
+ You need to set more options to build on Windows. See
:ref:`developers-cpp-windows` for details.
+Other options for build type are also possible:
+
+* ``Debug``: doesn't apply any optimization passes in the compiler and
+ leaves the symbol map in the binary.
+* ``FastDebug``
+* ``RelWithDebInfo``: applies compiler optimizations and it removes the symbol
+ map from the binary.
+* ``Release``: fast but less useful for debugging or profiling.
+* ``Profile_Gen``
+* ``Profile_Build``
Review comment:
If we list them, we should probably explain them as well (but I can't directly help with that ..)
And maybe we can also be opinionated on which ones to list and limit to the ones we think are useful to list, and don't necessarily need to list all of them.
(cc @westonpace)
--
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
[GitHub] [arrow] edponce commented on a change in pull request #11505: MINOR: [Docs][Python] Proposed changes for building pyarrow docs
Posted by GitBox <gi...@apache.org>.
edponce commented on a change in pull request #11505:
URL: https://github.com/apache/arrow/pull/11505#discussion_r734036143
##########
File path: docs/source/developers/cpp/building.rst
##########
@@ -121,9 +121,23 @@ argument is omitted then a release build will be produced.
.. note::
- You need to more options to build on Windows. See
+ You need to set more options to build on Windows. See
:ref:`developers-cpp-windows` for details.
+Other options for build type are also possible:
+
+* ``Debug``: doesn't apply any optimization passes in the compiler and
+ leaves the symbol map in the binary.
+* ``FastDebug``
+* ``RelWithDebInfo``: applies compiler optimizations and it removes the symbol
+ map from the binary.
+* ``Release``: fast but less useful for debugging or profiling.
+* ``Profile_Gen``
+* ``Profile_Build``
Review comment:
[This cmake files](https://github.com/apache/arrow/blob/master/cpp/cmake_modules/SetupCxxFlags.cmake#L575-L630) is where these flags are resolved.
* `Debug` sets `-g -O0` (and possibly `-ggdb`)
* `FastDebug` sets `-g -O1` (and possibly `-ggdb`)
* `RelWithDebInfo` does not sets anything in particular?
* `Release` sets `-gO3 -DNDEBUG`
* `Profile_Gen` sets `-gO3 -DNDEBUG -fprofile-generate`
* `Profile_Build` sets `-gO3 -DNDEBUG -fprofile-use`
`-ggdb` is set only if `ARROW_GGDB_DEBUG` is defined.
--
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
[GitHub] [arrow] jorisvandenbossche commented on a change in pull request #11505: MINOR: [Docs][Python] Proposed changes for building pyarrow docs
Posted by GitBox <gi...@apache.org>.
jorisvandenbossche commented on a change in pull request #11505:
URL: https://github.com/apache/arrow/pull/11505#discussion_r734338709
##########
File path: docs/source/developers/python.rst
##########
@@ -335,17 +334,27 @@ 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 it is advisable to delete the content of
+the build folder with command ``rm -rf *`` from the build folder.
+
Now, build pyarrow:
.. code-block:: shell
pushd arrow/python
export PYARROW_WITH_PARQUET=1
+ export PYARROW_WITH_FLIGHT=0
+ export PYARROW_WITH_GANDIVA=0
+ export PYARROW_WITH_ORC=0
+ export PYARROW_WITH_PLASMA=0
Review comment:
I would maybe leave something about it, but rephrase it the other way around: if you did build one of the optional components (in C++), you need to set the corresponding env variable to 1.
--
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
[GitHub] [arrow] AlenkaF commented on a change in pull request #11505: MINOR: [Docs][Python] Proposed changes for building pyarrow docs
Posted by GitBox <gi...@apache.org>.
AlenkaF commented on a change in pull request #11505:
URL: https://github.com/apache/arrow/pull/11505#discussion_r734268335
##########
File path: docs/source/developers/cpp/building.rst
##########
@@ -121,9 +121,23 @@ argument is omitted then a release build will be produced.
.. note::
- You need to more options to build on Windows. See
+ You need to set more options to build on Windows. See
:ref:`developers-cpp-windows` for details.
+Other options for build type are also possible:
+
+* ``Debug``: doesn't apply any optimization passes in the compiler and
+ leaves the symbol map in the binary.
+* ``FastDebug``
+* ``RelWithDebInfo``: applies compiler optimizations and it removes the symbol
+ map from the binary.
+* ``Release``: fast but less useful for debugging or profiling.
+* ``Profile_Gen``
+* ``Profile_Build``
Review comment:
Thanks @westonpace @edponce for writing it all again in more detail.
Yeah, that all makes sense. C++ developer will be familiar to use other types also. And for us beginners the main three are enough to choose from already. I will keep only first three and rephrase them as suggested.
--
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
[GitHub] [arrow] jorisvandenbossche commented on a change in pull request #11505: MINOR: [Docs][Python] Proposed changes for building pyarrow docs
Posted by GitBox <gi...@apache.org>.
jorisvandenbossche commented on a change in pull request #11505:
URL: https://github.com/apache/arrow/pull/11505#discussion_r744996931
##########
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:
Note that it would be `git clean -Xfd ../cpp/build` then, in case you are working from `arrow/python`, since this part is about the C++ build.
--
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
[GitHub] [arrow] pitrou commented on a change in pull request #11505: MINOR: [Docs][Python] Proposed changes for building pyarrow docs
Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #11505:
URL: https://github.com/apache/arrow/pull/11505#discussion_r744998946
##########
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:
You mean virtualenv will install the desired Python version for you?
--
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
[GitHub] [arrow] AlenkaF commented on a change in pull request #11505: MINOR: [Docs][Python] Proposed changes for building pyarrow docs
Posted by GitBox <gi...@apache.org>.
AlenkaF commented on a change in pull request #11505:
URL: https://github.com/apache/arrow/pull/11505#discussion_r734328137
##########
File path: docs/source/developers/python.rst
##########
@@ -335,17 +334,27 @@ 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 it is advisable to delete the content of
+the build folder with command ``rm -rf *`` from the build folder.
+
Now, build pyarrow:
.. code-block:: shell
pushd arrow/python
export PYARROW_WITH_PARQUET=1
+ export PYARROW_WITH_FLIGHT=0
+ export PYARROW_WITH_GANDIVA=0
+ export PYARROW_WITH_ORC=0
+ export PYARROW_WITH_PLASMA=0
Review comment:
Ok, git it. Should I remove this line from the docs then also?
`If you did not build one of the optional components, set the corresponding ``PYARROW_WITH_$COMPONENT`` environment variable to 0.`
--
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
[GitHub] [arrow] pitrou commented on a change in pull request #11505: MINOR: [Docs][Python] Proposed changes for building pyarrow docs
Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #11505:
URL: https://github.com/apache/arrow/pull/11505#discussion_r744972532
##########
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:
You would write `python3.9 -m venv pyarrow-dev`
--
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
[GitHub] [arrow] ursabot commented on pull request #11505: MINOR: [Docs][Python] Small updates to the documentation about building pyarrow
Posted by GitBox <gi...@apache.org>.
ursabot commented on pull request #11505:
URL: https://github.com/apache/arrow/pull/11505#issuecomment-975548505
Benchmark runs are scheduled for baseline = 92ee2952155070f7c0bfe3c05f9ccddebf7620db and contender = 14fa2cbb837bdfc5131b944df155804098299a91. 14fa2cbb837bdfc5131b944df155804098299a91 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Scheduled] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/b67a829e07924d4a9c2e8208eb223adc...f7254ed7db4142acbeb19b71c49f25be/)
[Scheduled] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/b1fac54660784ee399577220f904d538...6dc343de845c4a2189448fff7fb3a5e6/)
[Scheduled] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/0189017172fe49d68cd92d94658a78b9...82bac44a8d604197b8010f5e73151456/)
Supported benchmarks:
ursa-i9-9960x: langs = Python, R, JavaScript
ursa-thinkcentre-m75q: langs = C++, Java
ec2-t3-xlarge-us-east-2: cloud = True
--
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
[GitHub] [arrow] jorisvandenbossche commented on a change in pull request #11505: MINOR: [Docs][Python] Proposed changes for building pyarrow docs
Posted by GitBox <gi...@apache.org>.
jorisvandenbossche commented on a change in pull request #11505:
URL: https://github.com/apache/arrow/pull/11505#discussion_r744905393
##########
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:
(of course, assuming that you would then run `git clean` from the root of the repo)
##########
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:
The `rm -rf *` could also be changed to `rm -rf /cpp/build` to make this more explicit (and without needing to be run from that directory).
I think a possible advantage of `rm` vs `git clean` is that the clean command will remove everything else as well (eg if you have multiple build directories (although a beginning contributor won't have this), or sphinx doctrees / doc output, etc)
##########
File path: docs/source/developers/python.rst
##########
@@ -55,12 +55,15 @@ like so:
.. code-block:: shell
- pytest pyarrow
+ pytest arrow/python/pyarrow
Review comment:
That's certainly true, but _most_ of the other commands on this page are written assuming you are located just above the arrow repo. So for consistency (and for being more explicit), I think we can still update it.
And then any python programmer will still be able to understand how to edit it according to their location :)
(BTW, we actually ran into this because of the combination that the guide currently recommends to create a "pyarrow" virtualenv, and then doing `pytest pyarrow` following the other instructions, it tried to run the tests of the all packages in the virtualenv ... Now the virtualenv is renamed, that's of course less of an issue)
--
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
[GitHub] [arrow] jorisvandenbossche commented on a change in pull request #11505: MINOR: [Docs][Python] Proposed changes for building pyarrow docs
Posted by GitBox <gi...@apache.org>.
jorisvandenbossche commented on a change in pull request #11505:
URL: https://github.com/apache/arrow/pull/11505#discussion_r745505038
##########
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 generally delete the build folder itself, rather than delete its contents.
Yes, that was also the reason I mentioned to mention `rm -rf /cpp/build` instead of `rm -rf *` from inside that directory.
I agree that we can recommend the `git clean` way instead of `python setup.py clean` (your other inline comment below). But for explicitly deleting the C++ build directory (which is the topic here), wouldn't `rm -rf /cpp/build` be more explicit (also for understanding what is actually done / needed in this case)?
--
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
[GitHub] [arrow] AlenkaF commented on a change in pull request #11505: MINOR: [Docs][Python] Proposed changes for building pyarrow docs
Posted by GitBox <gi...@apache.org>.
AlenkaF commented on a change in pull request #11505:
URL: https://github.com/apache/arrow/pull/11505#discussion_r734287882
##########
File path: docs/source/developers/python.rst
##########
@@ -335,17 +334,27 @@ 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 it is advisable to delete the content of
+the build folder with command ``rm -rf *`` from the build folder.
+
Now, build pyarrow:
.. code-block:: shell
pushd arrow/python
export PYARROW_WITH_PARQUET=1
+ export PYARROW_WITH_FLIGHT=0
+ export PYARROW_WITH_GANDIVA=0
+ export PYARROW_WITH_ORC=0
+ export PYARROW_WITH_PLASMA=0
Review comment:
I agree and I did not use this extra fags from the start. But later I had some difficulties building and added them due to the line from the docs:
`If you did not build one of the optional components, set the corresponding
``PYARROW_WITH_$COMPONENT`` environment variable to 0.`
I am not sure it helped, but it might. And the docs are not really clear to me: the line I pasted states the 0 flags should be added, but are in fact not added in the example provided at the moment.
--
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
[GitHub] [arrow] jorisvandenbossche commented on a change in pull request #11505: MINOR: [Docs][Python] Proposed changes for building pyarrow docs
Posted by GitBox <gi...@apache.org>.
jorisvandenbossche commented on a change in pull request #11505:
URL: https://github.com/apache/arrow/pull/11505#discussion_r741747766
##########
File path: docs/source/developers/cpp/building.rst
##########
@@ -121,9 +121,21 @@ argument is omitted then a release build will be produced.
.. note::
- You need to more options to build on Windows. See
+ You need to set more options to build on Windows. See
:ref:`developers-cpp-windows` for details.
+Other options for build type are also possible:
+
+* ``Debug``: doesn't apply any compiler optimizations and leaves debugging
+ information in the binary.
+* ``RelWithDebInfo``: applies compiler optimizations while leaving debug
+ information in the binary.
+* ``Release``: applies compiler optimizations and removes debug information
+ from the binary.
+
+You can also run default build with flag `-DARROW_EXTRA_ERROR_CONTEXT=ON`, see
+:ref:`label-extra-debugging:`
Review comment:
```suggestion
:ref:`label-extra-debugging`
```
##########
File path: docs/source/developers/python.rst
##########
@@ -359,6 +364,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:
```suggestion
To run an editable pyarrow version run ``pip install -e . --no-build-isolation``
```
##########
File path: docs/source/developers/cpp/building.rst
##########
@@ -121,9 +121,21 @@ argument is omitted then a release build will be produced.
.. note::
- You need to more options to build on Windows. See
+ You need to set more options to build on Windows. See
:ref:`developers-cpp-windows` for details.
+Other options for build type are also possible:
+
+* ``Debug``: doesn't apply any compiler optimizations and leaves debugging
+ information in the binary.
+* ``RelWithDebInfo``: applies compiler optimizations while leaving debug
+ information in the binary.
+* ``Release``: applies compiler optimizations and removes debug information
+ from the binary.
+
+You can also run default build with flag `-DARROW_EXTRA_ERROR_CONTEXT=ON`, see
Review comment:
```suggestion
You can also run default build with flag ``-DARROW_EXTRA_ERROR_CONTEXT=ON``, see
```
(in restructuredtext we need two backticks for code)
##########
File path: docs/source/developers/python.rst
##########
@@ -306,6 +299,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 better understanding of the error in case the build fails add flag
+``-DCMAKE_BUILD_TYPE=debug``. Read more about build type in C++ building section
Review comment:
I think this flag doesn't help while building, but only afterwards when running? (so the "in case the build fails" is then not correct; it's still useful to point to the different build types though)
--
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
[GitHub] [arrow] pitrou commented on a change in pull request #11505: MINOR: [Docs][Python] Proposed changes for building pyarrow docs
Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #11505:
URL: https://github.com/apache/arrow/pull/11505#discussion_r744977070
##########
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:
Again, I'm assuming you're working inside the `python` directory :-)
But `git clean -Xfd .`, at least, won't provoke any disasters if you run it from the wrong 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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a change in pull request #11505: MINOR: [Docs][Python] Proposed changes for building pyarrow docs
Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #11505:
URL: https://github.com/apache/arrow/pull/11505#discussion_r745478308
##########
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:
Well, in this case the sentence can be about the Xcode Command Line Tools.
--
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
[GitHub] [arrow] ursabot edited a comment on pull request #11505: MINOR: [Docs][Python] Small updates to the documentation about building pyarrow
Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #11505:
URL: https://github.com/apache/arrow/pull/11505#issuecomment-975548505
Benchmark runs are scheduled for baseline = 92ee2952155070f7c0bfe3c05f9ccddebf7620db and contender = 14fa2cbb837bdfc5131b944df155804098299a91. 14fa2cbb837bdfc5131b944df155804098299a91 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/b67a829e07924d4a9c2e8208eb223adc...f7254ed7db4142acbeb19b71c49f25be/)
[Failed :arrow_down:0.0% :arrow_up:0.0%] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/b1fac54660784ee399577220f904d538...6dc343de845c4a2189448fff7fb3a5e6/)
[Finished :arrow_down:0.0% :arrow_up:0.0%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/0189017172fe49d68cd92d94658a78b9...82bac44a8d604197b8010f5e73151456/)
Supported benchmarks:
ursa-i9-9960x: langs = Python, R, JavaScript
ursa-thinkcentre-m75q: langs = C++, Java
ec2-t3-xlarge-us-east-2: cloud = True
--
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
[GitHub] [arrow] westonpace commented on a change in pull request #11505: MINOR: [Docs][Python] Proposed changes for building pyarrow docs
Posted by GitBox <gi...@apache.org>.
westonpace commented on a change in pull request #11505:
URL: https://github.com/apache/arrow/pull/11505#discussion_r734004326
##########
File path: docs/source/developers/cpp/building.rst
##########
@@ -121,9 +121,23 @@ argument is omitted then a release build will be produced.
.. note::
- You need to more options to build on Windows. See
+ You need to set more options to build on Windows. See
:ref:`developers-cpp-windows` for details.
+Other options for build type are also possible:
+
+* ``Debug``: doesn't apply any optimization passes in the compiler and
+ leaves the symbol map in the binary.
+* ``FastDebug``
+* ``RelWithDebInfo``: applies compiler optimizations and it removes the symbol
+ map from the binary.
+* ``Release``: fast but less useful for debugging or profiling.
+* ``Profile_Gen``
+* ``Profile_Build``
Review comment:
```
FastDebug - Same as Debug but with minimal optimizations enabled (-O1)
Profile_Gen - Same as Release but adds the -fprofile-generate flag
Profile_Build - Same as Release but adds the -fprofile-use flag
```
--
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
[GitHub] [arrow] westonpace commented on a change in pull request #11505: MINOR: [Docs][Python] Proposed changes for building pyarrow docs
Posted by GitBox <gi...@apache.org>.
westonpace commented on a change in pull request #11505:
URL: https://github.com/apache/arrow/pull/11505#discussion_r734075744
##########
File path: docs/source/developers/cpp/building.rst
##########
@@ -121,9 +121,23 @@ argument is omitted then a release build will be produced.
.. note::
- You need to more options to build on Windows. See
+ You need to set more options to build on Windows. See
:ref:`developers-cpp-windows` for details.
+Other options for build type are also possible:
+
+* ``Debug``: doesn't apply any optimization passes in the compiler and
+ leaves the symbol map in the binary.
+* ``FastDebug``
+* ``RelWithDebInfo``: applies compiler optimizations and it removes the symbol
+ map from the binary.
+* ``Release``: fast but less useful for debugging or profiling.
+* ``Profile_Gen``
+* ``Profile_Build``
Review comment:
As far as being opinionated I agree that Debug/RelWithDebInfo/Release are the most popular choices. C++ developers will also likely be familiar with those as they come builtin from cmake.
The other three (FastDebug/Profile_Gen/Profile_Build) are for fairly niche use cases and I'm not sure we really need to list them out here.
--
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
[GitHub] [arrow] jorisvandenbossche commented on a change in pull request #11505: MINOR: [Docs][Python] Small updates to the documentation about building pyarrow
Posted by GitBox <gi...@apache.org>.
jorisvandenbossche commented on a change in pull request #11505:
URL: https://github.com/apache/arrow/pull/11505#discussion_r749341810
##########
File path: docs/source/developers/cpp/building.rst
##########
@@ -121,9 +121,21 @@ argument is omitted then a release build will be produced.
.. note::
- You need to more options to build on Windows. See
+ You need to set more options to build on Windows. See
:ref:`developers-cpp-windows` for details.
+Several build types are possible:
+
+* ``Debug``: doesn't apply any compiler optimizations and adds debugging
+ information in the binary.
+* ``RelWithDebInfo``: applies compiler optimizations while adding debug
+ information in the binary.
+* ``Release``: applies compiler optimizations and removes debug information
+ from the binary.
+
+You can also run default build with flag ``-DARROW_EXTRA_ERROR_CONTEXT=ON``, see
+:ref:`label-extra-debugging`.
Review comment:
```suggestion
:ref:`cpp-extra-debugging`.
```
##########
File path: docs/source/developers/python.rst
##########
@@ -55,12 +55,16 @@ 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 you get import errors for ``pyarrow._lib`` or another PyArrow module when
+trying to run the tests run ``python -m pytest arrow/python/pyarrow`` and check
Review comment:
```suggestion
trying to run the tests, run ``python -m pytest arrow/python/pyarrow`` and check
```
##########
File path: docs/source/developers/cpp/building.rst
##########
@@ -424,6 +436,8 @@ this can be accomplished with the ``Threads`` built-in package:
find_package(Threads REQUIRED)
target_link_libraries(my_target PRIVATE Threads::Threads)
+.. _label-extra-debugging:
Review comment:
```suggestion
.. _cpp-extra-debugging:
```
(I propose to leave out "label", since this is indeed a label, but we generally don't include that in the label's name, but adding cpp might be more explicit when referencing this elsewhere)
##########
File path: docs/source/developers/python.rst
##########
@@ -306,6 +303,11 @@ adding flags with ``ON``:
Anything set to ``ON`` above can also be turned off. Note that some compression
libraries are needed for Parquet support.
+To enable C++ debugging information, pass the option ``-DCMAKE_BUILD_TYPE=debug``.
+
+.. seealso::
+ :ref:`cpp-building-building`.
Review comment:
Can you add a label to that section? (I think we are now using an auto-generated one based on the title of the section)
So with the `.. _label:` syntax, like you also added a label for the extra-debugging section.
--
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
[GitHub] [arrow] ursabot edited a comment on pull request #11505: MINOR: [Docs][Python] Small updates to the documentation about building pyarrow
Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #11505:
URL: https://github.com/apache/arrow/pull/11505#issuecomment-975548505
Benchmark runs are scheduled for baseline = 92ee2952155070f7c0bfe3c05f9ccddebf7620db and contender = 14fa2cbb837bdfc5131b944df155804098299a91. 14fa2cbb837bdfc5131b944df155804098299a91 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/b67a829e07924d4a9c2e8208eb223adc...f7254ed7db4142acbeb19b71c49f25be/)
[Failed :arrow_down:0.0% :arrow_up:0.0%] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/b1fac54660784ee399577220f904d538...6dc343de845c4a2189448fff7fb3a5e6/)
[Scheduled] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/0189017172fe49d68cd92d94658a78b9...82bac44a8d604197b8010f5e73151456/)
Supported benchmarks:
ursa-i9-9960x: langs = Python, R, JavaScript
ursa-thinkcentre-m75q: langs = C++, Java
ec2-t3-xlarge-us-east-2: cloud = True
--
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
[GitHub] [arrow] pitrou commented on a change in pull request #11505: MINOR: [Docs][Python] Proposed changes for building pyarrow docs
Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #11505:
URL: https://github.com/apache/arrow/pull/11505#discussion_r745481657
##########
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:
It's generally puzzling for people if you offer two different ways of doing the same thing. Also, here, it won't be doing exactly the same thing, which can be even more confusing. I would generally advocate `git clean -Xfd` because it should clean all build leftovers, including ones that may be outside of the build directory (it shouldn't happen in C++, but it can happen in Python).
##########
File path: docs/source/developers/python.rst
##########
@@ -55,12 +55,15 @@ like so:
.. code-block:: shell
- pytest pyarrow
+ pytest arrow/python/pyarrow
Review comment:
Hmm, I don't know, I never move back to the parent directory. Usually I would have one or several shells opened in the `cpp` directory and another one in the `python` directory, so I avoid back-and-forths.
So, yes, I would vote for staying in the `python` directory after building PyArrow.
--
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
[GitHub] [arrow] jorisvandenbossche commented on a change in pull request #11505: MINOR: [Docs][Python] Proposed changes for building pyarrow docs
Posted by GitBox <gi...@apache.org>.
jorisvandenbossche commented on a change in pull request #11505:
URL: https://github.com/apache/arrow/pull/11505#discussion_r745501894
##########
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:
> You mean virtualenv will install the desired Python version for you?
That was my assumption when writing that, but it might be you can only specify python versions you already have installed? (again, I am not familiar with virtualenv, so not the best person to know what we should recommend here :))
--
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
[GitHub] [arrow] jorisvandenbossche commented on a change in pull request #11505: MINOR: [Docs][Python] Proposed changes for building pyarrow docs
Posted by GitBox <gi...@apache.org>.
jorisvandenbossche commented on a change in pull request #11505:
URL: https://github.com/apache/arrow/pull/11505#discussion_r733783723
##########
File path: docs/source/developers/python.rst
##########
@@ -359,6 +368,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``
+ in the ``arrow/python``.
Review comment:
```suggestion
in the ``arrow/python`` directory.
```
##########
File path: docs/source/developers/python.rst
##########
@@ -335,17 +334,27 @@ 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 it is advisable to delete the content of
+the build folder with command ``rm -rf *`` from the build folder.
Review comment:
Maybe should clarify here this is only for if something goes wrong? (because afterwards, when you need to rebuild C++ (eg because you pulled the latest from master), you don't need to remove the build folder everytime)
--
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
[GitHub] [arrow] AlenkaF commented on a change in pull request #11505: MINOR: [Docs][Python] Proposed changes for building pyarrow docs
Posted by GitBox <gi...@apache.org>.
AlenkaF commented on a change in pull request #11505:
URL: https://github.com/apache/arrow/pull/11505#discussion_r734328137
##########
File path: docs/source/developers/python.rst
##########
@@ -335,17 +334,27 @@ 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 it is advisable to delete the content of
+the build folder with command ``rm -rf *`` from the build folder.
+
Now, build pyarrow:
.. code-block:: shell
pushd arrow/python
export PYARROW_WITH_PARQUET=1
+ export PYARROW_WITH_FLIGHT=0
+ export PYARROW_WITH_GANDIVA=0
+ export PYARROW_WITH_ORC=0
+ export PYARROW_WITH_PLASMA=0
Review comment:
Ok, git it. I will remove this line from the docs then also:
`If you did not build one of the optional components, set the corresponding ``PYARROW_WITH_$COMPONENT`` environment variable to 0.`
--
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
[GitHub] [arrow] jorisvandenbossche commented on pull request #11505: MINOR: [Docs][Python] Small updates to the documentation about building pyarrow
Posted by GitBox <gi...@apache.org>.
jorisvandenbossche commented on pull request #11505:
URL: https://github.com/apache/arrow/pull/11505#issuecomment-975551504
Thanks @AlenkaF !
--
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
[GitHub] [arrow] pitrou commented on a change in pull request #11505: MINOR: [Docs][Python] Proposed changes for building pyarrow docs
Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #11505:
URL: https://github.com/apache/arrow/pull/11505#discussion_r744976217
##########
File path: docs/source/developers/python.rst
##########
@@ -55,12 +55,15 @@ like so:
.. code-block:: shell
- pytest pyarrow
+ pytest arrow/python/pyarrow
Review comment:
I think the instructions should mimick the usual workflow, though. Is it common to run the PyArrow tests from the directory below the git checkout? I know I always work from the `python` 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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] AlenkaF commented on a change in pull request #11505: MINOR: [Docs][Python] Proposed changes for building pyarrow docs
Posted by GitBox <gi...@apache.org>.
AlenkaF commented on a change in pull request #11505:
URL: https://github.com/apache/arrow/pull/11505#discussion_r745384095
##########
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:
Adding both possibilities here also?
--
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
[GitHub] [arrow] pitrou commented on a change in pull request #11505: MINOR: [Docs][Python] Proposed changes for building pyarrow docs
Posted by GitBox <gi...@apache.org>.
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jorisvandenbossche commented on a change in pull request #11505: MINOR: [Docs][Python] Proposed changes for building pyarrow docs
Posted by GitBox <gi...@apache.org>.
jorisvandenbossche commented on a change in pull request #11505:
URL: https://github.com/apache/arrow/pull/11505#discussion_r744901329
##########
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:
(disclaimer: I am not very familiar with those tools, never use them myself)
I think virtualenv additionally allows to create an environment with a different python version? (in which case it might still be useful to use it here in the example)
--
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
[GitHub] [arrow] jorisvandenbossche commented on a change in pull request #11505: MINOR: [Docs][Python] Proposed changes for building pyarrow docs
Posted by GitBox <gi...@apache.org>.
jorisvandenbossche commented on a change in pull request #11505:
URL: https://github.com/apache/arrow/pull/11505#discussion_r744995855
##########
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:
But that requires you have python 3.9 installed? (and eg if you install python from python.org, I think you already are linked to python 3.10 by default)
--
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
[GitHub] [arrow] pitrou commented on a change in pull request #11505: MINOR: [Docs][Python] Proposed changes for building pyarrow docs
Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #11505:
URL: https://github.com/apache/arrow/pull/11505#discussion_r744999774
##########
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:
Hmm, that was not obvious to me. I would generally delete the build folder itself, rather than delete its contents.
--
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
[GitHub] [arrow] AlenkaF commented on a change in pull request #11505: MINOR: [Docs][Python] Proposed changes for building pyarrow docs
Posted by GitBox <gi...@apache.org>.
AlenkaF commented on a change in pull request #11505:
URL: https://github.com/apache/arrow/pull/11505#discussion_r745377131
##########
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:
Actually (I am a new Mac user so I am still learning) the easiest way would be to only install Xcode Command Line Tools via `xcode-select --install` or via Homebrew. Installing full Xcode is taking way to long, it stopped twice in my case and at the end I still wasn't sure if it worked.
Maybe it's just me but additional sentence can help ...
--
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
[GitHub] [arrow] jorisvandenbossche commented on a change in pull request #11505: MINOR: [Docs][Python] Proposed changes for building pyarrow docs
Posted by GitBox <gi...@apache.org>.
jorisvandenbossche commented on a change in pull request #11505:
URL: https://github.com/apache/arrow/pull/11505#discussion_r745500402
##########
File path: docs/source/developers/python.rst
##########
@@ -55,12 +55,15 @@ like so:
.. code-block:: shell
- pytest pyarrow
+ pytest arrow/python/pyarrow
Review comment:
Personally, I always stay in the root arrow repo (and have aliases for things I regularly need that have the path baked in or do the pushd/popd). So that's already a different workflow :)
> What we can do is stay in python folder after building pyarrow (I would than change the instructions there) and leave pytest as it was? What do u think?
I would personally leave that for a separate PR to discuss (it would impact almost every command here)
--
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
[GitHub] [arrow] westonpace commented on a change in pull request #11505: MINOR: [Docs][Python] Proposed changes for building pyarrow docs
Posted by GitBox <gi...@apache.org>.
westonpace commented on a change in pull request #11505:
URL: https://github.com/apache/arrow/pull/11505#discussion_r734004326
##########
File path: docs/source/developers/cpp/building.rst
##########
@@ -121,9 +121,23 @@ argument is omitted then a release build will be produced.
.. note::
- You need to more options to build on Windows. See
+ You need to set more options to build on Windows. See
:ref:`developers-cpp-windows` for details.
+Other options for build type are also possible:
+
+* ``Debug``: doesn't apply any optimization passes in the compiler and
+ leaves the symbol map in the binary.
+* ``FastDebug``
+* ``RelWithDebInfo``: applies compiler optimizations and it removes the symbol
+ map from the binary.
+* ``Release``: fast but less useful for debugging or profiling.
+* ``Profile_Gen``
+* ``Profile_Build``
Review comment:
```
FastDebug - Same as Debug but with minimal optimizations on (-O1)
Profile_Gen - Same as Release but adds -fprofile-generate profiling information into the binary
Profile_Build - Same as Release but adds -fprofile-use to consume profiling information from a previous Profile_Gen build.
```
--
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
[GitHub] [arrow] westonpace commented on a change in pull request #11505: MINOR: [Docs][Python] Proposed changes for building pyarrow docs
Posted by GitBox <gi...@apache.org>.
westonpace commented on a change in pull request #11505:
URL: https://github.com/apache/arrow/pull/11505#discussion_r734077844
##########
File path: docs/source/developers/python.rst
##########
@@ -106,8 +108,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 have XCode installed, no additional configuration is needed.
Review comment:
```suggestion
sufficient. All you need is to have XCode installed, no additional configuration is needed.
```
##########
File path: docs/source/developers/python.rst
##########
@@ -57,6 +57,8 @@ like so:
pytest pyarrow
+Be sure to run the command inside arrow/python folder.
Review comment:
```suggestion
Be sure to run the command inside the arrow/python folder.
```
##########
File path: docs/source/developers/cpp/building.rst
##########
@@ -121,9 +121,23 @@ argument is omitted then a release build will be produced.
.. note::
- You need to more options to build on Windows. See
+ You need to set more options to build on Windows. See
:ref:`developers-cpp-windows` for details.
+Other options for build type are also possible:
+
+* ``Debug``: doesn't apply any optimization passes in the compiler and
+ leaves the symbol map in the binary.
+* ``FastDebug``
+* ``RelWithDebInfo``: applies compiler optimizations and it removes the symbol
Review comment:
```suggestion
* ``RelWithDebInfo``: applies compiler optimizations while leaving debug information in the binary
```
##########
File path: docs/source/developers/python.rst
##########
@@ -335,17 +334,27 @@ 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 it is advisable to delete the content of
+the build folder with command ``rm -rf *`` from the build folder.
+
Now, build pyarrow:
.. code-block:: shell
pushd arrow/python
export PYARROW_WITH_PARQUET=1
+ export PYARROW_WITH_FLIGHT=0
+ export PYARROW_WITH_GANDIVA=0
+ export PYARROW_WITH_ORC=0
+ export PYARROW_WITH_PLASMA=0
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.
+``PYARROW_WITH_$COMPONENT`` environment variable to 0 as in this example.
+
+If you wish to delete pyarrow build before rebuilding navigate to the ``arrow/python``
+folder and run ``python setp.py clean``.
Review comment:
```suggestion
folder and run ``python setup.py clean``.
```
##########
File path: docs/source/developers/python.rst
##########
@@ -57,6 +57,8 @@ like so:
pytest pyarrow
+Be sure to run the command inside arrow/python folder.
Review comment:
Also, later (e.g. line 356 at the moment) you have:
```
``arrow/python``
```
I'm not sure if you want that here or not.
##########
File path: docs/source/developers/cpp/building.rst
##########
@@ -121,9 +121,23 @@ argument is omitted then a release build will be produced.
.. note::
- You need to more options to build on Windows. See
+ You need to set more options to build on Windows. See
:ref:`developers-cpp-windows` for details.
+Other options for build type are also possible:
+
+* ``Debug``: doesn't apply any optimization passes in the compiler and
+ leaves the symbol map in the binary.
Review comment:
```suggestion
* ``Debug``: doesn't apply any optimization passes in the compiler and
leaves debugging information in the binary.
```
"Symbol map" is a term I use a lot but it's probably too specific and not quite right. Debugging information is more generalized.
##########
File path: docs/source/developers/python.rst
##########
@@ -335,17 +334,27 @@ 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 it is advisable to delete the content of
+the build folder with command ``rm -rf *`` from the build folder.
+
Now, build pyarrow:
.. code-block:: shell
pushd arrow/python
export PYARROW_WITH_PARQUET=1
+ export PYARROW_WITH_FLIGHT=0
+ export PYARROW_WITH_GANDIVA=0
+ export PYARROW_WITH_ORC=0
+ export PYARROW_WITH_PLASMA=0
Review comment:
All of the `PYARROW_WITH` flags default to 0 so it is a bit odd to see them explicitly set to 0 here.
--
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
[GitHub] [arrow] westonpace commented on a change in pull request #11505: MINOR: [Docs][Python] Proposed changes for building pyarrow docs
Posted by GitBox <gi...@apache.org>.
westonpace commented on a change in pull request #11505:
URL: https://github.com/apache/arrow/pull/11505#discussion_r734004326
##########
File path: docs/source/developers/cpp/building.rst
##########
@@ -121,9 +121,23 @@ argument is omitted then a release build will be produced.
.. note::
- You need to more options to build on Windows. See
+ You need to set more options to build on Windows. See
:ref:`developers-cpp-windows` for details.
+Other options for build type are also possible:
+
+* ``Debug``: doesn't apply any optimization passes in the compiler and
+ leaves the symbol map in the binary.
+* ``FastDebug``
+* ``RelWithDebInfo``: applies compiler optimizations and it removes the symbol
+ map from the binary.
+* ``Release``: fast but less useful for debugging or profiling.
+* ``Profile_Gen``
+* ``Profile_Build``
Review comment:
```
FastDebug - Same as Debug but with minimal optimizations enabled (-O1)
Profile_Gen - Same as Release but adds -fprofile-generate profiling information into the binary
Profile_Build - Same as Release but adds -fprofile-use to consume profiling information from a previous Profile_Gen build.
```
--
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
[GitHub] [arrow] edponce commented on a change in pull request #11505: MINOR: [Docs][Python] Proposed changes for building pyarrow docs
Posted by GitBox <gi...@apache.org>.
edponce commented on a change in pull request #11505:
URL: https://github.com/apache/arrow/pull/11505#discussion_r734036143
##########
File path: docs/source/developers/cpp/building.rst
##########
@@ -121,9 +121,23 @@ argument is omitted then a release build will be produced.
.. note::
- You need to more options to build on Windows. See
+ You need to set more options to build on Windows. See
:ref:`developers-cpp-windows` for details.
+Other options for build type are also possible:
+
+* ``Debug``: doesn't apply any optimization passes in the compiler and
+ leaves the symbol map in the binary.
+* ``FastDebug``
+* ``RelWithDebInfo``: applies compiler optimizations and it removes the symbol
+ map from the binary.
+* ``Release``: fast but less useful for debugging or profiling.
+* ``Profile_Gen``
+* ``Profile_Build``
Review comment:
[This cmake files](https://github.com/apache/arrow/blob/master/cpp/cmake_modules/SetupCxxFlags.cmake#L575-L630) is where these flags are resolved.
* `Debug` sets `-g -O0` (and possibly `-ggdb`)
* `FastDebug` sets `-g -O1` (and possibly `-ggdb`)
* `RelWithDebInfo` does not sets anything in particular?
* `Release` sets `-gO3 -DNDEBUG`
* `Profile_Gen` sets `-gO3 -DNDEBUG -fprofile-generate`
* `Profile_Build` sets `-gO3 -DNDEBUG -fprofile-use`
`-ggdb` is set only if `ARROW_GGDB_DEBUG` is defined.
Some more detailed info is in the [*Bonus Material* section of this blog](https://bytefreaks.net/programming-2/cc-how-do-you-set-gdb-debug-flag-g-with-cmake).
--
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
[GitHub] [arrow] jorisvandenbossche commented on a change in pull request #11505: MINOR: [Docs][Python] Proposed changes for building pyarrow docs
Posted by GitBox <gi...@apache.org>.
jorisvandenbossche commented on a change in pull request #11505:
URL: https://github.com/apache/arrow/pull/11505#discussion_r734313856
##########
File path: docs/source/developers/python.rst
##########
@@ -335,17 +334,27 @@ 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 it is advisable to delete the content of
+the build folder with command ``rm -rf *`` from the build folder.
+
Now, build pyarrow:
.. code-block:: shell
pushd arrow/python
export PYARROW_WITH_PARQUET=1
+ export PYARROW_WITH_FLIGHT=0
+ export PYARROW_WITH_GANDIVA=0
+ export PYARROW_WITH_ORC=0
+ export PYARROW_WITH_PLASMA=0
Review comment:
Those should indeed default to 0, so normally it should not have had impact on your build ..
--
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
[GitHub] [arrow] AlenkaF commented on a change in pull request #11505: MINOR: [Docs][Python] Proposed changes for building pyarrow docs
Posted by GitBox <gi...@apache.org>.
AlenkaF commented on a change in pull request #11505:
URL: https://github.com/apache/arrow/pull/11505#discussion_r742589389
##########
File path: docs/source/developers/python.rst
##########
@@ -61,6 +61,9 @@ 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:
I added this part for pytest, if it makes sense to you all.
--
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
[GitHub] [arrow] jorisvandenbossche commented on a change in pull request #11505: MINOR: [Docs][Python] Proposed changes for building pyarrow docs
Posted by GitBox <gi...@apache.org>.
jorisvandenbossche commented on a change in pull request #11505:
URL: https://github.com/apache/arrow/pull/11505#discussion_r744993298
##########
File path: docs/source/developers/python.rst
##########
@@ -55,12 +55,15 @@ like so:
.. code-block:: shell
- pytest pyarrow
+ pytest arrow/python/pyarrow
Review comment:
> I think the instructions should mimick the usual workflow, though. Is it common to run the PyArrow tests from the directory below the git checkout?
I personally agree, I also never work from the directory where `git clone` is called (I typically work from the `arrow/` root of the repo). But currently all other instructions are written like this as well.
--
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
[GitHub] [arrow] AlenkaF commented on a change in pull request #11505: MINOR: [Docs][Python] Proposed changes for building pyarrow docs
Posted by GitBox <gi...@apache.org>.
AlenkaF commented on a change in pull request #11505:
URL: https://github.com/apache/arrow/pull/11505#discussion_r746421494
##########
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:
https://virtualenv.pypa.io/en/latest/cli_interface.html#p
--
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
[GitHub] [arrow] edponce commented on a change in pull request #11505: MINOR: [Docs][Python] Proposed changes for building pyarrow docs
Posted by GitBox <gi...@apache.org>.
edponce commented on a change in pull request #11505:
URL: https://github.com/apache/arrow/pull/11505#discussion_r746354035
##########
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:
General comment: The typical workflow to reset a build when using cmake is to remove the `<build dir>` not its contents.
##########
File path: docs/source/developers/cpp/building.rst
##########
@@ -121,9 +121,21 @@ argument is omitted then a release build will be produced.
.. note::
- You need to more options to build on Windows. See
+ You need to set more options to build on Windows. See
:ref:`developers-cpp-windows` for details.
+Other options for build type are also possible:
+
+* ``Debug``: doesn't apply any compiler optimizations and leaves debugging
+ information in the binary.
+* ``RelWithDebInfo``: applies compiler optimizations while leaving debug
+ information in the binary.
Review comment:
I suggest to change `leaves debugging information` to `adds debugging information`.
--
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
[GitHub] [arrow] AlenkaF commented on a change in pull request #11505: MINOR: [Docs][Python] Proposed changes for building pyarrow docs
Posted by GitBox <gi...@apache.org>.
AlenkaF commented on a change in pull request #11505:
URL: https://github.com/apache/arrow/pull/11505#discussion_r746408563
##########
File path: docs/source/developers/python.rst
##########
@@ -55,12 +55,15 @@ like so:
.. code-block:: shell
- pytest pyarrow
+ pytest arrow/python/pyarrow
Review comment:
Ok.
--
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
[GitHub] [arrow] ursabot edited a comment on pull request #11505: MINOR: [Docs][Python] Small updates to the documentation about building pyarrow
Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #11505:
URL: https://github.com/apache/arrow/pull/11505#issuecomment-975548505
Benchmark runs are scheduled for baseline = 92ee2952155070f7c0bfe3c05f9ccddebf7620db and contender = 14fa2cbb837bdfc5131b944df155804098299a91. 14fa2cbb837bdfc5131b944df155804098299a91 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/b67a829e07924d4a9c2e8208eb223adc...f7254ed7db4142acbeb19b71c49f25be/)
[Scheduled] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/b1fac54660784ee399577220f904d538...6dc343de845c4a2189448fff7fb3a5e6/)
[Scheduled] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/0189017172fe49d68cd92d94658a78b9...82bac44a8d604197b8010f5e73151456/)
Supported benchmarks:
ursa-i9-9960x: langs = Python, R, JavaScript
ursa-thinkcentre-m75q: langs = C++, Java
ec2-t3-xlarge-us-east-2: cloud = True
--
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
[GitHub] [arrow] jorisvandenbossche closed pull request #11505: MINOR: [Docs][Python] Small updates to the documentation about building pyarrow
Posted by GitBox <gi...@apache.org>.
jorisvandenbossche closed pull request #11505:
URL: https://github.com/apache/arrow/pull/11505
--
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
[GitHub] [arrow] AlenkaF commented on a change in pull request #11505: MINOR: [Docs][Python] Proposed changes for building pyarrow docs
Posted by GitBox <gi...@apache.org>.
AlenkaF commented on a change in pull request #11505:
URL: https://github.com/apache/arrow/pull/11505#discussion_r743444184
##########
File path: docs/source/developers/python.rst
##########
@@ -61,6 +61,9 @@ 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:
@jorisvandenbossche what do you think?
--
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
[GitHub] [arrow] jorisvandenbossche commented on a change in pull request #11505: MINOR: [Docs][Python] Proposed changes for building pyarrow docs
Posted by GitBox <gi...@apache.org>.
jorisvandenbossche commented on a change in pull request #11505:
URL: https://github.com/apache/arrow/pull/11505#discussion_r741747766
##########
File path: docs/source/developers/cpp/building.rst
##########
@@ -121,9 +121,21 @@ argument is omitted then a release build will be produced.
.. note::
- You need to more options to build on Windows. See
+ You need to set more options to build on Windows. See
:ref:`developers-cpp-windows` for details.
+Other options for build type are also possible:
+
+* ``Debug``: doesn't apply any compiler optimizations and leaves debugging
+ information in the binary.
+* ``RelWithDebInfo``: applies compiler optimizations while leaving debug
+ information in the binary.
+* ``Release``: applies compiler optimizations and removes debug information
+ from the binary.
+
+You can also run default build with flag `-DARROW_EXTRA_ERROR_CONTEXT=ON`, see
+:ref:`label-extra-debugging:`
Review comment:
```suggestion
:ref:`label-extra-debugging`
```
##########
File path: docs/source/developers/python.rst
##########
@@ -359,6 +364,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:
```suggestion
To run an editable pyarrow version run ``pip install -e . --no-build-isolation``
```
##########
File path: docs/source/developers/cpp/building.rst
##########
@@ -121,9 +121,21 @@ argument is omitted then a release build will be produced.
.. note::
- You need to more options to build on Windows. See
+ You need to set more options to build on Windows. See
:ref:`developers-cpp-windows` for details.
+Other options for build type are also possible:
+
+* ``Debug``: doesn't apply any compiler optimizations and leaves debugging
+ information in the binary.
+* ``RelWithDebInfo``: applies compiler optimizations while leaving debug
+ information in the binary.
+* ``Release``: applies compiler optimizations and removes debug information
+ from the binary.
+
+You can also run default build with flag `-DARROW_EXTRA_ERROR_CONTEXT=ON`, see
Review comment:
```suggestion
You can also run default build with flag ``-DARROW_EXTRA_ERROR_CONTEXT=ON``, see
```
(in restructuredtext we need two backticks for code)
##########
File path: docs/source/developers/python.rst
##########
@@ -306,6 +299,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 better understanding of the error in case the build fails add flag
+``-DCMAKE_BUILD_TYPE=debug``. Read more about build type in C++ building section
Review comment:
I think this flag doesn't help while building, but only afterwards when running? (so the "in case the build fails" is then not correct; it's still useful to point to the different build types though)
--
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
[GitHub] [arrow] AlenkaF commented on a change in pull request #11505: MINOR: [Docs][Python] Proposed changes for building pyarrow docs
Posted by GitBox <gi...@apache.org>.
AlenkaF commented on a change in pull request #11505:
URL: https://github.com/apache/arrow/pull/11505#discussion_r743444184
##########
File path: docs/source/developers/python.rst
##########
@@ -61,6 +61,9 @@ 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:
@jorisvandenbossche what do you think?
##########
File path: docs/source/developers/python.rst
##########
@@ -61,6 +61,9 @@ 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:
@jorisvandenbossche what do you think?
--
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
[GitHub] [arrow] jorisvandenbossche commented on a change in pull request #11505: MINOR: [Docs][Python] Proposed changes for building pyarrow docs
Posted by GitBox <gi...@apache.org>.
jorisvandenbossche commented on a change in pull request #11505:
URL: https://github.com/apache/arrow/pull/11505#discussion_r741747766
##########
File path: docs/source/developers/cpp/building.rst
##########
@@ -121,9 +121,21 @@ argument is omitted then a release build will be produced.
.. note::
- You need to more options to build on Windows. See
+ You need to set more options to build on Windows. See
:ref:`developers-cpp-windows` for details.
+Other options for build type are also possible:
+
+* ``Debug``: doesn't apply any compiler optimizations and leaves debugging
+ information in the binary.
+* ``RelWithDebInfo``: applies compiler optimizations while leaving debug
+ information in the binary.
+* ``Release``: applies compiler optimizations and removes debug information
+ from the binary.
+
+You can also run default build with flag `-DARROW_EXTRA_ERROR_CONTEXT=ON`, see
+:ref:`label-extra-debugging:`
Review comment:
```suggestion
:ref:`label-extra-debugging`
```
##########
File path: docs/source/developers/python.rst
##########
@@ -359,6 +364,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:
```suggestion
To run an editable pyarrow version run ``pip install -e . --no-build-isolation``
```
##########
File path: docs/source/developers/cpp/building.rst
##########
@@ -121,9 +121,21 @@ argument is omitted then a release build will be produced.
.. note::
- You need to more options to build on Windows. See
+ You need to set more options to build on Windows. See
:ref:`developers-cpp-windows` for details.
+Other options for build type are also possible:
+
+* ``Debug``: doesn't apply any compiler optimizations and leaves debugging
+ information in the binary.
+* ``RelWithDebInfo``: applies compiler optimizations while leaving debug
+ information in the binary.
+* ``Release``: applies compiler optimizations and removes debug information
+ from the binary.
+
+You can also run default build with flag `-DARROW_EXTRA_ERROR_CONTEXT=ON`, see
Review comment:
```suggestion
You can also run default build with flag ``-DARROW_EXTRA_ERROR_CONTEXT=ON``, see
```
(in restructuredtext we need two backticks for code)
##########
File path: docs/source/developers/python.rst
##########
@@ -306,6 +299,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 better understanding of the error in case the build fails add flag
+``-DCMAKE_BUILD_TYPE=debug``. Read more about build type in C++ building section
Review comment:
I think this flag doesn't help while building, but only afterwards when running? (so the "in case the build fails" is then not correct; it's still useful to point to the different build types though)
--
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
[GitHub] [arrow] jorisvandenbossche commented on a change in pull request #11505: MINOR: [Docs][Python] Proposed changes for building pyarrow docs
Posted by GitBox <gi...@apache.org>.
jorisvandenbossche commented on a change in pull request #11505:
URL: https://github.com/apache/arrow/pull/11505#discussion_r741747766
##########
File path: docs/source/developers/cpp/building.rst
##########
@@ -121,9 +121,21 @@ argument is omitted then a release build will be produced.
.. note::
- You need to more options to build on Windows. See
+ You need to set more options to build on Windows. See
:ref:`developers-cpp-windows` for details.
+Other options for build type are also possible:
+
+* ``Debug``: doesn't apply any compiler optimizations and leaves debugging
+ information in the binary.
+* ``RelWithDebInfo``: applies compiler optimizations while leaving debug
+ information in the binary.
+* ``Release``: applies compiler optimizations and removes debug information
+ from the binary.
+
+You can also run default build with flag `-DARROW_EXTRA_ERROR_CONTEXT=ON`, see
+:ref:`label-extra-debugging:`
Review comment:
```suggestion
:ref:`label-extra-debugging`
```
##########
File path: docs/source/developers/python.rst
##########
@@ -359,6 +364,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:
```suggestion
To run an editable pyarrow version run ``pip install -e . --no-build-isolation``
```
##########
File path: docs/source/developers/cpp/building.rst
##########
@@ -121,9 +121,21 @@ argument is omitted then a release build will be produced.
.. note::
- You need to more options to build on Windows. See
+ You need to set more options to build on Windows. See
:ref:`developers-cpp-windows` for details.
+Other options for build type are also possible:
+
+* ``Debug``: doesn't apply any compiler optimizations and leaves debugging
+ information in the binary.
+* ``RelWithDebInfo``: applies compiler optimizations while leaving debug
+ information in the binary.
+* ``Release``: applies compiler optimizations and removes debug information
+ from the binary.
+
+You can also run default build with flag `-DARROW_EXTRA_ERROR_CONTEXT=ON`, see
Review comment:
```suggestion
You can also run default build with flag ``-DARROW_EXTRA_ERROR_CONTEXT=ON``, see
```
(in restructuredtext we need two backticks for code)
##########
File path: docs/source/developers/python.rst
##########
@@ -306,6 +299,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 better understanding of the error in case the build fails add flag
+``-DCMAKE_BUILD_TYPE=debug``. Read more about build type in C++ building section
Review comment:
I think this flag doesn't help while building, but only afterwards when running? (so the "in case the build fails" is then not correct; it's still useful to point to the different build types though)
--
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
[GitHub] [arrow] pitrou commented on a change in pull request #11505: MINOR: [Docs][Python] Proposed changes for building pyarrow docs
Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #11505:
URL: https://github.com/apache/arrow/pull/11505#discussion_r744831895
##########
File path: docs/source/developers/cpp/building.rst
##########
@@ -121,9 +121,21 @@ argument is omitted then a release build will be produced.
.. note::
- You need to more options to build on Windows. See
+ You need to set more options to build on Windows. See
:ref:`developers-cpp-windows` for details.
+Other options for build type are also possible:
Review comment:
I would say: "Several build types are possible"
--
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
[GitHub] [arrow] AlenkaF commented on a change in pull request #11505: MINOR: [Docs][Python] Proposed changes for building pyarrow docs
Posted by GitBox <gi...@apache.org>.
AlenkaF commented on a change in pull request #11505:
URL: https://github.com/apache/arrow/pull/11505#discussion_r745370110
##########
File path: docs/source/developers/python.rst
##########
@@ -55,12 +55,15 @@ like so:
.. code-block:: shell
- pytest pyarrow
+ pytest arrow/python/pyarrow
Review comment:
Yes, currently the instructions are written from the folder `repos`, which is above `arrow`. Even when building `pyarrow` you move from `repos` to `python` folder in `arrow` and than back to `repos` again.
I am sure there are standard ways of working but for someone already dealing with build and scale of Arrow library I think this simple things should be written explicitly and without adding more confusion.
What we can do is stay in python folder after building pyarrow (I would than change the instructions there) and leave pytest as it was? What do u think?
--
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
[GitHub] [arrow] AlenkaF commented on a change in pull request #11505: MINOR: [Docs][Python] Proposed changes for building pyarrow docs
Posted by GitBox <gi...@apache.org>.
AlenkaF commented on a change in pull request #11505:
URL: https://github.com/apache/arrow/pull/11505#discussion_r745383659
##########
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:
What if we add both possible ways? The beginning contributor would pick one if it will be written understandably or try both. For advanced users it is clear that both cases work so no harm done?
--
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