-
Notifications
You must be signed in to change notification settings - Fork 3.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ARROW-17868: [C++][Python] Restore the ARROW_PYTHON CMake option #14273
Changes from all commits
4686e50
9450955
18a6620
b7b48a8
a4c64de
4e609b7
dfffd6f
e6b084c
954f1ad
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -96,25 +96,27 @@ cmake \ | |
-DARROW_BUILD_SHARED=ON \ | ||
-DARROW_BUILD_STATIC=OFF \ | ||
-DARROW_BUILD_TESTS=OFF \ | ||
-DARROW_COMPUTE=ON \ | ||
-DARROW_CSV=ON \ | ||
-DARROW_DATASET=${ARROW_DATASET} \ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we just hardcode this to ON? (the ARROW_PYTHON=ON would have ensured that) I don't think we want to create wheels without dataset enabled? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I think so but |
||
-DARROW_DEPENDENCY_SOURCE="VCPKG" \ | ||
-DARROW_DEPENDENCY_USE_SHARED=OFF \ | ||
-DARROW_FILESYSTEM=ON \ | ||
-DARROW_FLIGHT=${ARROW_FLIGHT} \ | ||
-DARROW_GANDIVA=${ARROW_GANDIVA} \ | ||
-DARROW_GCS=${ARROW_GCS} \ | ||
-DARROW_HDFS=${ARROW_HDFS} \ | ||
-DARROW_JEMALLOC=${ARROW_JEMALLOC} \ | ||
-DARROW_JSON=ON \ | ||
-DARROW_MIMALLOC=${ARROW_MIMALLOC} \ | ||
-DARROW_ORC=${ARROW_ORC} \ | ||
-DARROW_PACKAGE_KIND="python-wheel-macos" \ | ||
-DARROW_PARQUET=${ARROW_PARQUET} \ | ||
-DPARQUET_REQUIRE_ENCRYPTION=${PARQUET_REQUIRE_ENCRYPTION} \ | ||
-DARROW_PLASMA=${ARROW_PLASMA} \ | ||
-DARROW_PYTHON=ON \ | ||
-DARROW_RPATH_ORIGIN=ON \ | ||
-DARROW_SUBSTRAIT=${ARROW_SUBSTRAIT} \ | ||
-DARROW_S3=${ARROW_S3} \ | ||
-DARROW_SIMD_LEVEL=${ARROW_SIMD_LEVEL} \ | ||
-DARROW_SUBSTRAIT=${ARROW_SUBSTRAIT} \ | ||
-DARROW_TENSORFLOW=${ARROW_TENSORFLOW} \ | ||
-DARROW_USE_CCACHE=ON \ | ||
-DARROW_WITH_BROTLI=${ARROW_WITH_BROTLI} \ | ||
|
@@ -129,9 +131,9 @@ cmake \ | |
-DCMAKE_INSTALL_PREFIX=${build_dir}/install \ | ||
-DCMAKE_OSX_ARCHITECTURES=${CMAKE_OSX_ARCHITECTURES} \ | ||
-DCMAKE_UNITY_BUILD=${CMAKE_UNITY_BUILD} \ | ||
-DOPENSSL_USE_STATIC_LIBS=ON \ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems unrelated to the other changes. Can you give a short reasoning for it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. This is unrelated to
I just found this by sorting CMake options. So I mix this change into this pull request. Sorry. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No need to be sorry. I was just wondering about the reasoning |
||
-DORC_PROTOBUF_EXECUTABLE=${VCPKG_ROOT}/installed/${VCPKG_TARGET_TRIPLET}/tools/protobuf/protoc \ | ||
-DORC_SOURCE=BUNDLED \ | ||
-DPARQUET_REQUIRE_ENCRYPTION=${PARQUET_REQUIRE_ENCRYPTION} \ | ||
-DVCPKG_MANIFEST_MODE=OFF \ | ||
-DVCPKG_TARGET_TRIPLET=${VCPKG_TARGET_TRIPLET} \ | ||
-G ${CMAKE_GENERATOR} \ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -117,17 +117,61 @@ | |
"ARROW_GANDIVA": "ON" | ||
} | ||
}, | ||
{ | ||
"name": "features-python-minimal", | ||
"inherits": [ | ||
"features-minimal" | ||
], | ||
"hidden": true, | ||
"cacheVariables": { | ||
"ARROW_COMPUTE": "ON", | ||
"ARROW_CSV": "ON", | ||
"ARROW_FILESYSTEM": "ON", | ||
"ARROW_JSON": "ON" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I haven't yet used the presets myself, but shall we include cc @wjones127 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I'm fine not including datasets if it's not required for the Python build. In most workflows with CMakePresets.json, I think we expect developers to create their own presets in CMakeUserPresets.json, which will inherit from the provided once. (See an example here) Once this is merged, I can send a notice to the mailing list with instructions on how to transition to the new presets. (I need to update my blog post anyways.) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
It makes sense. I added no |
||
} | ||
}, | ||
{ | ||
"name": "features-python", | ||
"inherits": [ | ||
"features-main" | ||
], | ||
"hidden": true, | ||
"cacheVariables": { | ||
"ARROW_COMPUTE": "ON", | ||
"ARROW_CSV": "ON", | ||
"ARROW_DATASET": "ON", | ||
"ARROW_FILESYSTEM": "ON", | ||
"ARROW_JSON": "ON", | ||
"ARROW_ORC": "ON" | ||
} | ||
}, | ||
{ | ||
"name": "features-python-maximal", | ||
"inherits": [ | ||
"features-cuda", | ||
"features-filesystems", | ||
"features-flight", | ||
"features-gandiva", | ||
"features-main", | ||
"features-python-minimal" | ||
], | ||
"hidden": true, | ||
"cacheVariables": { | ||
"ARROW_ORC": "ON", | ||
"PARQUET_REQUIRE_ENCRYPTION": "ON" | ||
} | ||
}, | ||
{ | ||
"name": "features-maximal", | ||
"inherits": [ | ||
"features-main", | ||
"features-cuda", | ||
"features-filesystems", | ||
"features-flight", | ||
"features-gandiva" | ||
"features-gandiva", | ||
"features-python-maximal" | ||
], | ||
"hidden": true, | ||
"displayName": "Debug build with everything enabled (except benchmarks and CUDA)", | ||
"cacheVariables": { | ||
"ARROW_BUILD_EXAMPLES": "ON", | ||
"ARROW_BUILD_UTILITIES": "ON", | ||
|
@@ -185,6 +229,24 @@ | |
"displayName": "Debug build with tests and Gandiva", | ||
"cacheVariables": {} | ||
}, | ||
{ | ||
"name": "ninja-debug-python-minimal", | ||
"inherits": ["base-debug", "features-python-minimal"], | ||
"displayName": "Debug build for PyArrow with minimal features", | ||
"cacheVariables": {} | ||
}, | ||
{ | ||
"name": "ninja-debug-python", | ||
"inherits": ["base-debug", "features-python"], | ||
"displayName": "Debug build for PyArrow with common features (for backward compatibility)", | ||
"cacheVariables": {} | ||
}, | ||
{ | ||
"name": "ninja-debug-python-maximal", | ||
"inherits": ["base-debug", "features-python-maximal"], | ||
"displayName": "Debug build for PyArrow with everything enabled (except CUDA)", | ||
"cacheVariables": {} | ||
}, | ||
{ | ||
"name": "ninja-debug-maximal", | ||
"inherits": ["base-debug", "features-maximal"], | ||
|
@@ -228,6 +290,24 @@ | |
"displayName": "Release build with Gandiva", | ||
"cacheVariables": {} | ||
}, | ||
{ | ||
"name": "ninja-release-python-minimal", | ||
"inherits": ["base-release", "features-python-minimal"], | ||
"displayName": "Release build for PyArrow with minimal features", | ||
"cacheVariables": {} | ||
}, | ||
{ | ||
"name": "ninja-release-python", | ||
"inherits": ["base-release", "features-python"], | ||
"displayName": "Release build for PyArrow with common features (for backward compatibility)", | ||
"cacheVariables": {} | ||
}, | ||
{ | ||
"name": "ninja-release-python-maximal", | ||
"inherits": ["base-release", "features-python-maximal"], | ||
"displayName": "Release build for PyArrow with everything enabled (except CUDA)", | ||
"cacheVariables": {} | ||
}, | ||
{ | ||
"name": "ninja-release-maximal", | ||
"inherits": ["base-release", "features-maximal"], | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is all this required for R? AFAIR, Python is only used to test PyArrow-R interoperability here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nealrichardson Do you know what components are needed here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that we can disable
ARROW_HDFS
. Other components seems necessary. I'll try it.