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

[GitHub] [arrow] AlenkaF opened a new pull request, #14074: ARROW-17579: [Python] PYARROW_CXXFLAGS ignored?

AlenkaF opened a new pull request, #14074:
URL: https://github.com/apache/arrow/pull/14074

   This PR adds:
   - `PYARROW_CXXFLAGS` variable to PyArrow C++ and Pyarrow cmake builds (both defined in `setup.py`)
   - a common flags section to `pyarrow/src/CMakeLists.txt` where `CXX_COMMON_FLAGS` and `PYARROW_CXXFLAGS` are added to the `CMAKE_CXX_FLAGS` variable.
   
   Other flags defined in Arrow C++ should be included in PyArrow C++ and PyArrow with `include(SetupCxxFlags)`.
   
   I am not sure if additional flags that are defined in PyArrow, see: https://github.com/apache/arrow/blob/43670af02f0913580fd20e26006fd550d6fdf2da/python/CMakeLists.txt#L134-L174 are needed in PyArrow C++ as they were not used when PyArrow C++ was included in Arrow C++?
   cc @pitrou 


-- 
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 merged pull request #14074: ARROW-17579: [Python] PYARROW_CXXFLAGS ignored?

Posted by GitBox <gi...@apache.org>.
AlenkaF merged PR #14074:
URL: https://github.com/apache/arrow/pull/14074


-- 
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 diff in pull request #14074: ARROW-17579: [Python] PYARROW_CXXFLAGS ignored?

Posted by GitBox <gi...@apache.org>.
pitrou commented on code in PR #14074:
URL: https://github.com/apache/arrow/pull/14074#discussion_r969611403


##########
python/setup.py:
##########
@@ -337,9 +338,10 @@ def _run_cmake(self, pyarrow_cpp_home):
             static_lib_option = ''
 
             cmake_options = [
-                '-DPYTHON_EXECUTABLE=%s' % sys.executable,
-                '-DPython3_EXECUTABLE=%s' % sys.executable,
+                '-DPYTHON_EXECUTABLE=' + str(sys.executable),
+                '-DPython3_EXECUTABLE=' + str(sys.executable),

Review Comment:
   Hmm, I see the same idiom is used above actually...



-- 
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 diff in pull request #14074: ARROW-17579: [Python] PYARROW_CXXFLAGS ignored?

Posted by GitBox <gi...@apache.org>.
pitrou commented on code in PR #14074:
URL: https://github.com/apache/arrow/pull/14074#discussion_r969610984


##########
python/setup.py:
##########
@@ -337,9 +338,10 @@ def _run_cmake(self, pyarrow_cpp_home):
             static_lib_option = ''
 
             cmake_options = [
-                '-DPYTHON_EXECUTABLE=%s' % sys.executable,
-                '-DPython3_EXECUTABLE=%s' % sys.executable,
+                '-DPYTHON_EXECUTABLE=' + str(sys.executable),
+                '-DPython3_EXECUTABLE=' + str(sys.executable),

Review Comment:
   Nit: it should already be a string:
   ```suggestion
                   '-DPYTHON_EXECUTABLE=' + sys.executable,
                   '-DPython3_EXECUTABLE=' + sys.executable,
   ```



-- 
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 #14074: ARROW-17579: [Python] PYARROW_CXXFLAGS ignored?

Posted by GitBox <gi...@apache.org>.
ursabot commented on PR #14074:
URL: https://github.com/apache/arrow/pull/14074#issuecomment-1247942619

   ['Python', 'R'] benchmarks have high level of regressions.
   [test-mac-arm](https://conbench.ursa.dev/compare/runs/50a033a17f714fdaba6b6ec1bba6913d...21f43a99eb6f4c3798b0c85e37361763/)
   


-- 
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 pull request #14074: ARROW-17579: [Python] PYARROW_CXXFLAGS ignored?

Posted by GitBox <gi...@apache.org>.
pitrou commented on PR #14074:
URL: https://github.com/apache/arrow/pull/14074#issuecomment-1245395301

   @AlenkaF You might try this patch to fix the AppVeyor error:
   ```diff
   diff --git a/python/pyarrow/_compute.pyx b/python/pyarrow/_compute.pyx
   index b9594d90e..2aa65e75c 100644
   --- a/python/pyarrow/_compute.pyx
   +++ b/python/pyarrow/_compute.pyx
   @@ -2613,7 +2613,7 @@ def register_scalar_function(func, function_name, function_doc, in_types,
            raise TypeError(
                "in_types must be a dictionary of DataType")
    
   -    c_arity = CArity(num_args, func_spec.varargs)
   +    c_arity = CArity(<int> num_args, func_spec.varargs)
    
        if "summary" not in function_doc:
            raise ValueError("Function doc must contain a summary")
   ```


-- 
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] github-actions[bot] commented on pull request #14074: ARROW-17579: [Python] PYARROW_CXXFLAGS ignored?

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #14074:
URL: https://github.com/apache/arrow/pull/14074#issuecomment-1240559461

   https://issues.apache.org/jira/browse/ARROW-17579


-- 
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 #14074: ARROW-17579: [Python] PYARROW_CXXFLAGS ignored?

Posted by GitBox <gi...@apache.org>.
ursabot commented on PR #14074:
URL: https://github.com/apache/arrow/pull/14074#issuecomment-1247942360

   Benchmark runs are scheduled for baseline = 81f3945d46c072c18f6eb60ec139cd5ca514379b and contender = 9c628b703f253402e0e45abf243cc3f7241f58a4. 9c628b703f253402e0e45abf243cc3f7241f58a4 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/31320ac25b484c2cabfde93de16abd5e...d85d89e08e104db5adaf876fd1f2aad7/)
   [Failed :arrow_down:0.24% :arrow_up:0.03%] [test-mac-arm](https://conbench.ursa.dev/compare/runs/50a033a17f714fdaba6b6ec1bba6913d...21f43a99eb6f4c3798b0c85e37361763/)
   [Failed :arrow_down:0.0% :arrow_up:0.0%] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/a101cc0c1720402a9af215cbbbb81305...0a89a8d4c04347b0a683586ff257eb2a/)
   [Finished :arrow_down:0.25% :arrow_up:0.0%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/6088e33905ef4654930630eeb49e319f...9c788484a9984e1aa2a639c7c76d98f7/)
   Buildkite builds:
   [Finished] [`9c628b70` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/1480)
   [Finished] [`9c628b70` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/1496)
   [Failed] [`9c628b70` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/1480)
   [Finished] [`9c628b70` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/1493)
   [Finished] [`81f3945d` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/1479)
   [Failed] [`81f3945d` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/1495)
   [Failed] [`81f3945d` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/1479)
   [Finished] [`81f3945d` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/1492)
   Supported benchmarks:
   ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
   test-mac-arm: Supported benchmark langs: C++, Python, R
   ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
   ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
   


-- 
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 diff in pull request #14074: ARROW-17579: [Python] PYARROW_CXXFLAGS ignored?

Posted by GitBox <gi...@apache.org>.
AlenkaF commented on code in PR #14074:
URL: https://github.com/apache/arrow/pull/14074#discussion_r969625966


##########
python/setup.py:
##########
@@ -337,9 +338,10 @@ def _run_cmake(self, pyarrow_cpp_home):
             static_lib_option = ''
 
             cmake_options = [
-                '-DPYTHON_EXECUTABLE=%s' % sys.executable,
-                '-DPython3_EXECUTABLE=%s' % sys.executable,
+                '-DPYTHON_EXECUTABLE=' + str(sys.executable),
+                '-DPython3_EXECUTABLE=' + str(sys.executable),

Review Comment:
   Yeah, I am OK with removing it everywhere.



-- 
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 pull request #14074: ARROW-17579: [Python] PYARROW_CXXFLAGS ignored?

Posted by GitBox <gi...@apache.org>.
AlenkaF commented on PR #14074:
URL: https://github.com/apache/arrow/pull/14074#issuecomment-1246643766

   @pitrou I have added the suggestions and will merge this PR later today if there is no objection.
   
   Note: Travis issue has a [JIRA already](https://issues.apache.org/jira/browse/ARROW-17715).


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