ARROW-2574: [Python] Add Cython and Python code coverage

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

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

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 \
@@ -152,7 +173,7 @@ 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
+  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)

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"

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)

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}

python/.coveragerc

@@ -0,0 +1,19 @@
+# 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
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+[run]
+plugins = Cython.Coverage

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")
+    endif()
+
     target_link_libraries(${module_name} ${LINK_LIBS})
+
+    # Generated files will be moved to the right directory by setup.py.
 endforeach(module)

python/COVERAGE-NOTES-REMOVEME.md

@@ -0,0 +1,33 @@
+# 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
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+To build Cython modules with coverage:
+```
+export PYARROW_CMAKE_OPTIONS="$PYARROW_CMAKE_OPTIONS -DCYTHON_FLAGS='-X linetrace=True'"
+
+python setup.py build_ext --inplace
+```
+
+To collect coverage data:
+```
+coverage run -m pytest pyarrow/tests/
+```
+
+To display a coverage report:
+```
+coverage report
+```

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