You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by ap...@apache.org on 2018/05/17 14:21:55 UTC
[arrow] branch master updated: ARROW-2574: [Python] Add Cython and
Python code coverage
This is an automated email from the ASF dual-hosted git repository.
apitrou pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/master by this push:
new 7764bc8 ARROW-2574: [Python] Add Cython and Python code coverage
7764bc8 is described below
commit 7764bc88902d5aec4db6261d7674334fbb0abfc0
Author: Antoine Pitrou <an...@python.org>
AuthorDate: Thu May 17 16:21:47 2018 +0200
ARROW-2574: [Python] Add Cython and Python code coverage
After spending a non-trivial time wrestling with Cython and our build system, we're now able to generate and upload Python and Cython coverage results as part of a Travis-CI run (in addition to C++ coverage).
Author: Antoine Pitrou <an...@python.org>
Closes #2050 from pitrou/ARROW-2574-cython-coverage and squashes the following commits:
4553185 <Antoine Pitrou> Remove leftover
b1212a4 <Antoine Pitrou> Silence "unknown warning option" error on clang
e1a5b4a <Antoine Pitrou> Disable ORC when building benchmarks
06b0665 <Antoine Pitrou> Try to fix Sphinx doc building
9b41d24 <Antoine Pitrou> Add nogil tracing
4014951 <Antoine Pitrou> ARROW-2574: Add Cython and Python code coverage
---
ci/travis_env_common.sh | 1 +
ci/travis_script_cpp.sh | 2 +-
ci/travis_script_python.sh | 58 ++++++++++++++++------
ci/travis_upload_cpp_coverage.sh | 2 +-
cpp/CMakeLists.txt | 4 ++
cpp/cmake_modules/SetupCxxFlags.cmake | 5 ++
cpp/cmake_modules/UseCython.cmake | 22 +++++---
.../.coveragerc | 16 +-----
python/.gitignore | 7 +--
python/CMakeLists.txt | 16 +++++-
python/setup.py | 45 ++++++++++++++---
11 files changed, 127 insertions(+), 51 deletions(-)
diff --git a/ci/travis_env_common.sh b/ci/travis_env_common.sh
index a4ae5bd..568070e 100755
--- a/ci/travis_env_common.sh
+++ b/ci/travis_env_common.sh
@@ -31,6 +31,7 @@ export ARROW_RUST_DIR=${TRAVIS_BUILD_DIR}/rust
if [ $ARROW_TRAVIS_COVERAGE == "1" ]; then
export ARROW_CPP_COVERAGE_FILE=${TRAVIS_BUILD_DIR}/coverage.info
+ export ARROW_PYTHON_COVERAGE_FILE=${TRAVIS_BUILD_DIR}/.coverage
fi
export CPP_BUILD_DIR=$TRAVIS_BUILD_DIR/cpp-build
diff --git a/ci/travis_script_cpp.sh b/ci/travis_script_cpp.sh
index 02e58cd..eedca98 100755
--- a/ci/travis_script_cpp.sh
+++ b/ci/travis_script_cpp.sh
@@ -28,7 +28,7 @@ ctest -j2 --output-on-failure -L unittest
popd
# Capture C++ coverage info (we wipe the build dir in travis_script_python.sh)
-if [ $ARROW_TRAVIS_COVERAGE == "1" ]; then
+if [ "$ARROW_TRAVIS_COVERAGE" == "1" ]; then
pushd $TRAVIS_BUILD_DIR
lcov --quiet --directory . --capture --no-external --output-file $ARROW_CPP_COVERAGE_FILE
popd
diff --git a/ci/travis_script_python.sh b/ci/travis_script_python.sh
index aaca3fd..3c0ea6d 100755
--- a/ci/travis_script_python.sh
+++ b/ci/travis_script_python.sh
@@ -85,25 +85,31 @@ popd
# python-test isn't run by travis_script_cpp.sh, exercise it here
$ARROW_CPP_BUILD_DIR/$ARROW_BUILD_TYPE/python-test
-# Other stuff pip install
pushd $ARROW_PYTHON_DIR
+# Other stuff pip install
+pip install -q -r requirements.txt
if [ "$PYTHON_VERSION" == "2.7" ]; then
pip install -q futures
fi
+if [ "$ARROW_TRAVIS_COVERAGE" == "1" ]; then
+ export PYARROW_GENERATE_COVERAGE=1
+ pip install -q coverage
+fi
export PYARROW_BUILD_TYPE=$ARROW_BUILD_TYPE
+export PYARROW_WITH_PARQUET=1
+export PYARROW_WITH_PLASMA=1
+export PYARROW_WITH_ORC=1
-pip install -q -r requirements.txt
-python setup.py build_ext -q --with-parquet --with-plasma --with-orc\
- install -q --single-version-externally-managed --record=record.text
-popd
+python setup.py build_ext -q --inplace
+# Basic sanity checks
python -c "import pyarrow.parquet"
python -c "import pyarrow.plasma"
python -c "import pyarrow.orc"
-if [ $ARROW_TRAVIS_VALGRIND == "1" ]; then
+if [ "$ARROW_TRAVIS_VALGRIND" == "1" ]; then
export PLASMA_VALGRIND=1
fi
@@ -115,20 +121,35 @@ if [ $TRAVIS_OS_NAME == "linux" ]; then
sudo bash -c "echo 20000 > /proc/sys/vm/nr_hugepages"
fi
-PYARROW_PATH=$CONDA_PREFIX/lib/python$PYTHON_VERSION/site-packages/pyarrow
-python -m pytest -r sxX --durations=15 $PYARROW_PATH --parquet
+# Need to run tests from the source tree for Cython coverage and conftest.py
+if [ "$ARROW_TRAVIS_COVERAGE" == "1" ]; then
+ # Output Python coverage data in a persistent place
+ export COVERAGE_FILE=$ARROW_PYTHON_COVERAGE_FILE
+ coverage run --append -m pytest -r sxX --durations=15 --parquet pyarrow/tests
+else
+ python -m pytest -r sxX --durations=15 --parquet pyarrow/tests
+fi
-# Capture C++ coverage info and combine with previous coverage file
-if [ $ARROW_TRAVIS_COVERAGE == "1" ]; then
+if [ "$ARROW_TRAVIS_COVERAGE" == "1" ]; then
+ # Check Cython coverage was correctly captured in $COVERAGE_FILE
+ coverage report -i --include="*/lib.pyx"
+ coverage report -i --include="*/memory.pxi"
+ coverage report -i --include="*/_parquet.pyx"
+ # Generate XML file for CodeCov
+ coverage xml -i -o $TRAVIS_BUILD_DIR/coverage.xml
+ # Capture C++ coverage info and combine with previous coverage file
pushd $TRAVIS_BUILD_DIR
lcov --quiet --directory . --capture --no-external --output-file coverage-python-tests.info
lcov --add-tracefile coverage-python-tests.info \
--add-tracefile $ARROW_CPP_COVERAGE_FILE \
--output-file $ARROW_CPP_COVERAGE_FILE
rm coverage-python-tests.info
- popd
+ popd # $TRAVIS_BUILD_DIR
fi
+popd # $ARROW_PYTHON_DIR
+
+
if [ "$ARROW_TRAVIS_PYTHON_DOCS" == "1" ] && [ "$PYTHON_VERSION" == "3.6" ]; then
# Build documentation once
conda install -y -q \
@@ -138,9 +159,12 @@ if [ "$ARROW_TRAVIS_PYTHON_DOCS" == "1" ] && [ "$PYTHON_VERSION" == "3.6" ]; the
sphinx \
sphinx_bootstrap_theme
- pushd $ARROW_PYTHON_DIR/doc
+ pushd $ARROW_PYTHON_DIR
+ # For autodoc, make sure PyArrow is installed
+ python setup.py install -q --single-version-externally-managed --record=record.text
+ cd doc
sphinx-build -q -b html -d _build/doctrees -W source _build/html
- popd
+ popd # $ARROW_PYTHON_DIR
fi
if [ "$ARROW_TRAVIS_PYTHON_BENCHMARKS" == "1" ] && [ "$PYTHON_VERSION" == "3.6" ]; then
@@ -152,12 +176,16 @@ if [ "$ARROW_TRAVIS_PYTHON_BENCHMARKS" == "1" ] && [ "$PYTHON_VERSION" == "3.6"
source activate pyarrow_asv
pip install -q git+https://github.com/pitrou/asv.git@customize_commands
- pushd $TRAVIS_BUILD_DIR/python
+ export PYARROW_WITH_PARQUET=0
+ export PYARROW_WITH_PLASMA=1
+ export PYARROW_WITH_ORC=0
+
+ pushd $ARROW_PYTHON_DIR
# Workaround for https://github.com/airspeed-velocity/asv/issues/631
git fetch --depth=100 origin master:master
# Generate machine information (mandatory)
asv machine --yes
# Run benchmarks on the changeset being tested
asv run --no-pull --show-stderr --quick HEAD^!
- popd
+ popd # $ARROW_PYTHON_DIR
fi
diff --git a/ci/travis_upload_cpp_coverage.sh b/ci/travis_upload_cpp_coverage.sh
index 20b7e1b..2b11c5e 100755
--- a/ci/travis_upload_cpp_coverage.sh
+++ b/ci/travis_upload_cpp_coverage.sh
@@ -23,7 +23,7 @@ source $TRAVIS_BUILD_DIR/ci/travis_env_common.sh
pushd $TRAVIS_BUILD_DIR
-# Display summary
+# Display C++ coverage summary
lcov --list $ARROW_CPP_COVERAGE_FILE
# Upload report to CodeCov
bash <(curl -s https://codecov.io/bash) || echo "Codecov did not collect coverage reports"
diff --git a/cpp/CMakeLists.txt b/cpp/CMakeLists.txt
index 0051c73..521e685 100644
--- a/cpp/CMakeLists.txt
+++ b/cpp/CMakeLists.txt
@@ -227,6 +227,10 @@ if("${CMAKE_SOURCE_DIR}" STREQUAL "${CMAKE_CURRENT_SOURCE_DIR}")
"Build with GRPC"
OFF)
+ option(ARROW_GENERATE_COVERAGE
+ "Build with C++ code coverage enabled"
+ OFF)
+
option(ARROW_VERBOSE_THIRDPARTY_BUILD
"If off, output from ExternalProjects will be logged to files rather than shown"
OFF)
diff --git a/cpp/cmake_modules/SetupCxxFlags.cmake b/cpp/cmake_modules/SetupCxxFlags.cmake
index 6387cfc..6803127 100644
--- a/cpp/cmake_modules/SetupCxxFlags.cmake
+++ b/cpp/cmake_modules/SetupCxxFlags.cmake
@@ -172,6 +172,11 @@ if ("${COMPILER_FAMILY}" STREQUAL "msvc")
set(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} /wd4800")
endif()
+# Avoid clang error when an unknown warning flag is passed
+if ("${COMPILER_FAMILY}" STREQUAL "clang")
+ set(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} -Wno-unknown-warning-option")
+endif()
+
# if build warning flags is set, add to CXX_COMMON_FLAGS
if (BUILD_WARNING_FLAGS)
# Use BUILD_WARNING_FLAGS with BUILD_WARNING_LEVEL=everything to disable
diff --git a/cpp/cmake_modules/UseCython.cmake b/cpp/cmake_modules/UseCython.cmake
index a1cfac6..6e6718a 100644
--- a/cpp/cmake_modules/UseCython.cmake
+++ b/cpp/cmake_modules/UseCython.cmake
@@ -68,10 +68,11 @@ mark_as_advanced( CYTHON_ANNOTATE CYTHON_NO_DOCSTRINGS CYTHON_FLAGS)
find_package( PythonLibsNew REQUIRED )
-set( CYTHON_CXX_EXTENSION "cxx" )
+# (using another C++ extension breaks coverage)
+set( CYTHON_CXX_EXTENSION "cpp" )
set( CYTHON_C_EXTENSION "c" )
-# Create a *.c or *.cxx file from a *.pyx file.
+# Create a *.c or *.cpp file from a *.pyx file.
# Input the generated file basename. The generate files will put into the variable
# placed in the "generated_files" argument. Finally all the *.py and *.pyx files.
function( compile_pyx _name pyx_target_name generated_files pyx_file)
@@ -93,6 +94,8 @@ function( compile_pyx _name pyx_target_name generated_files pyx_file)
endif()
get_source_file_property( pyx_location ${pyx_file} LOCATION )
+ set ( output_file "${_name}.${extension}" )
+
# Set additional flags.
if( CYTHON_ANNOTATE )
set( annotate_arg "--annotate" )
@@ -113,11 +116,11 @@ function( compile_pyx _name pyx_target_name generated_files pyx_file)
get_source_file_property( property_is_public ${pyx_file} CYTHON_PUBLIC )
get_source_file_property( property_is_api ${pyx_file} CYTHON_API )
if( ${property_is_api} )
- set( _generated_files "${_name}.${extension}" "${_name}.h" "${name}_api.h")
+ set( _generated_files "${output_file}" "${_name}.h" "${name}_api.h")
elseif( ${property_is_public} )
- set( _generated_files "${_name}.${extension}" "${_name}.h")
+ set( _generated_files "${output_file}" "${_name}.h")
else()
- set( _generated_files "${_name}.${extension}")
+ set( _generated_files "${output_file}")
endif()
set_source_files_properties( ${_generated_files} PROPERTIES GENERATED TRUE )
@@ -131,12 +134,15 @@ function( compile_pyx _name pyx_target_name generated_files pyx_file)
# Add the command to run the compiler.
add_custom_target(${pyx_target_name}
- COMMAND ${PYTHON_EXECUTABLE} -m cython ${cxx_arg} ${include_directory_arg}
+ COMMAND ${PYTHON_EXECUTABLE} -m cython ${cxx_arg}
${annotate_arg} ${no_docstrings_arg} ${cython_debug_arg}
${CYTHON_FLAGS}
- --output-file "${_name}.${extension}" ${pyx_location}
+ # Necessary for Cython code coverage
+ --working ${CMAKE_CURRENT_SOURCE_DIR}
+ --output-file "${CMAKE_CURRENT_BINARY_DIR}/${output_file}"
+ "${CMAKE_CURRENT_SOURCE_DIR}/${pyx_file}"
DEPENDS ${pyx_location}
- # do not specify byproducts for now since they don't work with the older
+ # Do not specify byproducts for now since they don't work with the older
# version of cmake available in the apt repositories.
#BYPRODUCTS ${_generated_files}
COMMENT ${comment}
diff --git a/ci/travis_upload_cpp_coverage.sh b/python/.coveragerc
old mode 100755
new mode 100644
similarity index 73%
copy from ci/travis_upload_cpp_coverage.sh
copy to python/.coveragerc
index 20b7e1b..f5dc6e3
--- a/ci/travis_upload_cpp_coverage.sh
+++ b/python/.coveragerc
@@ -1,5 +1,3 @@
-#!/usr/bin/env bash
-
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
@@ -17,15 +15,5 @@
# specific language governing permissions and limitations
# under the License.
-set -e
-
-source $TRAVIS_BUILD_DIR/ci/travis_env_common.sh
-
-pushd $TRAVIS_BUILD_DIR
-
-# Display summary
-lcov --list $ARROW_CPP_COVERAGE_FILE
-# Upload report to CodeCov
-bash <(curl -s https://codecov.io/bash) || echo "Codecov did not collect coverage reports"
-
-popd
+[run]
+plugins = Cython.Coverage
diff --git a/python/.gitignore b/python/.gitignore
index 6a63e5a..0633bb4 100644
--- a/python/.gitignore
+++ b/python/.gitignore
@@ -29,10 +29,11 @@ build
dist
# Egg metadata
*.egg-info
-# coverage
+# Coverage
.coverage
coverage.xml
-# cache
+htmlcov
+# Cache
.cache
# benchmark working dir
@@ -43,4 +44,4 @@ pyarrow/_table_api.h
manylinux1/arrow
# plasma store
-pyarrow/plasma_store
\ No newline at end of file
+pyarrow/plasma_store
diff --git a/python/CMakeLists.txt b/python/CMakeLists.txt
index 99194c2..01b7b54 100644
--- a/python/CMakeLists.txt
+++ b/python/CMakeLists.txt
@@ -79,6 +79,9 @@ if("${CMAKE_SOURCE_DIR}" STREQUAL "${CMAKE_CURRENT_SOURCE_DIR}")
option(PYARROW_BUNDLE_BOOST
"Bundle the Boost libraries when we bundle Arrow C++"
OFF)
+ option(PYARROW_GENERATE_COVERAGE
+ "Build with Cython code coverage enabled"
+ OFF)
set(PYARROW_CXXFLAGS "" CACHE STRING
"Compiler flags to append when compiling Arrow")
endif()
@@ -106,7 +109,7 @@ if (NOT MSVC)
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-omit-frame-pointer")
# Suppress Cython warnings
- set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-unused-variable")
+ set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-unused-variable -Wno-maybe-uninitialized")
else()
# MSVC version of -Wno-return-type-c-linkage
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /wd4190")
@@ -420,6 +423,10 @@ endif()
# Setup and build Cython modules
############################################################
+if (PYARROW_GENERATE_COVERAGE)
+ set(CYTHON_FLAGS "${CYTHON_FLAGS}" "-Xlinetrace=True")
+endif()
+
foreach(module ${CYTHON_EXTENSIONS})
string(REPLACE "." ";" directories ${module})
list(GET directories -1 module_name)
@@ -460,5 +467,12 @@ foreach(module ${CYTHON_EXTENSIONS})
INSTALL_RPATH ${module_install_rpath})
endif()
+ if (PYARROW_GENERATE_COVERAGE)
+ set_target_properties(${module_name} PROPERTIES
+ COMPILE_DEFINITIONS "CYTHON_TRACE=1;CYTHON_TRACE_NOGIL=1")
+ endif()
+
target_link_libraries(${module_name} ${LINK_LIBS})
+
+ # Generated files will be moved to the right directory by setup.py.
endforeach(module)
diff --git a/python/setup.py b/python/setup.py
index 2e0d982..7a8118b 100644
--- a/python/setup.py
+++ b/python/setup.py
@@ -102,6 +102,7 @@ class build_ext(_build_ext):
('with-static-boost', None, 'link boost statically'),
('with-plasma', None, 'build the Plasma extension'),
('with-orc', None, 'build the ORC extension'),
+ ('generate-coverage', None, 'enable Cython code coverage'),
('bundle-boost', None,
'bundle the (shared) Boost libraries'),
('bundle-arrow-cpp', None,
@@ -135,6 +136,8 @@ class build_ext(_build_ext):
os.environ.get('PYARROW_WITH_PLASMA', '0'))
self.with_orc = strtobool(
os.environ.get('PYARROW_WITH_ORC', '0'))
+ self.generate_coverage = strtobool(
+ os.environ.get('PYARROW_GENERATE_COVERAGE', '0'))
self.bundle_arrow_cpp = strtobool(
os.environ.get('PYARROW_BUNDLE_ARROW_CPP', '0'))
self.bundle_boost = strtobool(
@@ -197,6 +200,9 @@ class build_ext(_build_ext):
cmake_options.append('-DPYARROW_CXXFLAGS={0}'
.format(self.cmake_cxxflags))
+ if self.generate_coverage:
+ cmake_options.append('-DPYARROW_GENERATE_COVERAGE=on')
+
if self.bundle_arrow_cpp:
cmake_options.append('-DPYARROW_BUNDLE_ARROW_CPP=ON')
# ARROW-1090: work around CMake rough edges
@@ -303,13 +309,30 @@ class build_ext(_build_ext):
raise RuntimeError('pyarrow C-extension failed to build:',
os.path.abspath(built_path))
+ cpp_generated_path = self.get_ext_generated_cpp_source(name)
+ if not os.path.exists(cpp_generated_path):
+ raise RuntimeError('expected to find generated C++ file '
+ 'in {0!r}'.format(cpp_generated_path))
+
+ # The destination path to move the generated C++ source to
+ # (for Cython source coverage)
+ cpp_path = pjoin(build_lib, self._get_build_dir(),
+ os.path.basename(cpp_generated_path))
+ if os.path.exists(cpp_path):
+ os.remove(cpp_path)
+
+ # The destination path to move the built C extension to
ext_path = pjoin(build_lib, self._get_cmake_ext_path(name))
if os.path.exists(ext_path):
os.remove(ext_path)
self.mkpath(os.path.dirname(ext_path))
+
+ print('Moving generated C++ source', cpp_generated_path,
+ 'to build path', cpp_path)
+ shutil.move(cpp_generated_path, cpp_path)
print('Moving built C-extension', built_path,
'to build path', ext_path)
- shutil.move(self.get_ext_built(name), ext_path)
+ shutil.move(built_path, ext_path)
self._found_names.append(name)
if os.path.exists(self.get_ext_built_api_header(name)):
@@ -321,7 +344,7 @@ class build_ext(_build_ext):
build_py = self.get_finalized_command('build_py')
source = os.path.join(self.build_type, "plasma_store")
target = os.path.join(build_lib,
- build_py.get_package_dir('pyarrow'),
+ self._get_build_dir(),
"plasma_store")
shutil.move(source, target)
@@ -334,19 +357,25 @@ class build_ext(_build_ext):
return True
return False
- def _get_inplace_dir(self):
- pass
-
- def _get_cmake_ext_path(self, name):
+ def _get_build_dir(self):
# Get the package directory from build_py
build_py = self.get_finalized_command('build_py')
- package_dir = build_py.get_package_dir('pyarrow')
+ return build_py.get_package_dir('pyarrow')
+
+ def _get_cmake_ext_path(self, name):
# This is the name of the arrow C-extension
suffix = sysconfig.get_config_var('EXT_SUFFIX')
if suffix is None:
suffix = sysconfig.get_config_var('SO')
filename = name + suffix
- return pjoin(package_dir, filename)
+ return pjoin(self._get_build_dir(), filename)
+
+ def get_ext_generated_cpp_source(self, name):
+ if sys.platform == 'win32':
+ head, tail = os.path.split(name)
+ return pjoin(head, tail + ".cpp")
+ else:
+ return pjoin(name + ".cpp")
def get_ext_built_api_header(self, name):
if sys.platform == 'win32':
--
To stop receiving notification emails like this one, please contact
apitrou@apache.org.