ARROW-932: [Python] Fix MSVC compiler warnings, build Python with /WX and -Werror in CI#913

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.

Sign up for GitHub

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

Closed
wesm wants to merge 6 commits into from
Closed

ARROW-932: [Python] Fix MSVC compiler warnings, build Python with /WX and -Werror in CI #913

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c32ee09
Remove print statement. Disable MSVC 4190 warning
wesm Jul 29, 2017
5740f00
Add PYARROW_CXXFLAGS option, fix MSVC compiler warnings
wesm Jul 29, 2017
2e8f105
typo
wesm Jul 29, 2017
b5a6d9a
Supress another clang warning
wesm Jul 30, 2017
dedcbb9
Fix typo
wesm Jul 30, 2017
9534ae9
Only pass PYARROW_CXXFLAGS when set
wesm Jul 30, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions ci/msvc-build.bat
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -114,6 +114,9 @@ popd
@rem see PARQUET-1018

pushd python

set PYARROW_CXXFLAGS=/WX
python setup.py build_ext --inplace --with-parquet --bundle-arrow-cpp bdist_wheel || exit /B
py.test pyarrow -v -s --parquet || exit /B

popd
1 change: 1 addition & 0 deletions ci/travis_script_python.sh
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -23,6 +23,7 @@ source $TRAVIS_BUILD_DIR/ci/travis_env_common.sh
export ARROW_HOME=$ARROW_CPP_INSTALL
export PARQUET_HOME=$TRAVIS_BUILD_DIR/parquet-env
export LD_LIBRARY_PATH=$ARROW_HOME/lib:$PARQUET_HOME/lib:$LD_LIBRARY_PATH
export PYARROW_CXXFLAGS="-Werror"

build_parquet_cpp() {
export PARQUET_ARROW_VERSION=$(git rev-parse HEAD)
Expand Down
11 changes: 11 additions & 0 deletions python/CMakeLists.txt
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -57,6 +57,8 @@ if("${CMAKE_SOURCE_DIR}" STREQUAL "${CMAKE_CURRENT_SOURCE_DIR}")
option(PYARROW_BUNDLE_ARROW_CPP
"Bundle the Arrow C++ libraries"
OFF)
set(PYARROW_CXXFLAGS "" CACHE STRING
"Compiler flags to append when compiling Arrow")
endif()

find_program(CCACHE_FOUND ccache)
Expand All@@ -75,13 +77,21 @@ include(CompilerInfo)

# Add common flags
set(CMAKE_CXX_FLAGS "${CXX_COMMON_FLAGS} ${CMAKE_CXX_FLAGS}")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${PYARROW_CXXFLAGS}")

if (NOT MSVC)
# Enable perf and other tools to work properly
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-omit-frame-pointer")

# Suppress Cython warnings
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-unused-variable")
else()
# MSVC version of -Wno-return-type-c-linkage
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /wd4190")

# Cython generates some bitshift expressions that MSVC does not like in
# __Pyx_PyFloat_DivideObjC
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /wd4293")
endif()

if ("${COMPILER_FAMILY}" STREQUAL "clang")
Expand All@@ -95,6 +105,7 @@ if ("${COMPILER_FAMILY}" STREQUAL "clang")
# Cython warnings in clang
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-parentheses-equality")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-constant-logical-operand")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-missing-declarations")

# We have public Cython APIs which return C++ types, which are in an extern
# "C" blog (no symbol mangling) and clang doesn't like this
Expand Down
1 change: 0 additions & 1 deletion python/doc/source/development.rst
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -267,7 +267,6 @@ Now, we build and install Arrow C++ libraries
-DCMAKE_INSTALL_PREFIX=%ARROW_HOME% ^
-DCMAKE_BUILD_TYPE=Release ^
-DARROW_BUILD_TESTS=off ^
-DARROW_ZLIB_VENDORED=off ^
-DARROW_PYTHON=on ..
cmake --build . --target INSTALL --config Release
cd ..\..
Expand Down
2 changes: 1 addition & 1 deletion python/pyarrow/_parquet.pyx
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -636,7 +636,7 @@ cdef class ParquetWriter:
elif row_group_size == 0:
raise ValueError('Row group size cannot be 0')

cdef int c_row_group_size = row_group_size
cdef int64_t c_row_group_size = row_group_size

with nogil:
check_status(self.writer.get()
Expand Down
2 changes: 1 addition & 1 deletion python/pyarrow/includes/libarrow.pxd
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -369,7 +369,7 @@ cdef extern from "arrow/api.h" namespace "arrow" nogil:
shared_ptr[CTable]* table)

int num_columns()
int num_rows()
int64_t num_rows()

c_bool Equals(const CTable& other)

Expand Down
2 changes: 1 addition & 1 deletion python/pyarrow/io.pxi
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -923,7 +923,7 @@ cdef class _HdfsClient:
cdef c_string c_path = tobytes(path)
with nogil:
check_status(self.client.get()
.Delete(c_path, recursive))
.Delete(c_path, recursive == 1))

def open(self, path, mode='rb', buffer_size=None, replication=None,
default_block_size=None):
Expand Down
1 change: 0 additions & 1 deletion python/pyarrow/scalar.pxi
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -169,7 +169,6 @@ cdef class Time64Value(ArrayValue):
CTime64Type* dtype = <CTime64Type*> ap.type().get()

cdef int64_t val = ap.Value(self.index)
print(val)
if dtype.unit() == TimeUnit_MICRO:
return (datetime.datetime(1970, 1, 1) +
datetime.timedelta(microseconds=val)).time()
Expand Down
10 changes: 5 additions & 5 deletions python/pyarrow/table.pxi
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -286,7 +286,7 @@ cdef int _schema_from_arrays(
c_string c_name
vector[shared_ptr[CField]] fields
shared_ptr[CDataType] type_
int K = len(arrays)
Py_ssize_t K = len(arrays)

fields.resize(K)

Expand DownExpand Up@@ -733,7 +733,7 @@ cdef class Table:
vector[shared_ptr[CColumn]] columns
shared_ptr[CSchema] schema
shared_ptr[CTable] table
size_t K = len(arrays)
int i, K = <int> len(arrays)

_schema_from_arrays(arrays, names, metadata, &schema)

Expand DownExpand Up@@ -841,7 +841,7 @@ cdef class Table:
self._check_nullptr()
return pyarrow_wrap_schema(self.table.schema())

def column(self, int64_t i):
def column(self, int i):
"""
Select a column by its numeric index.

Expand All@@ -855,8 +855,8 @@ cdef class Table:
"""
cdef:
Column column = Column()
int64_t num_columns = self.num_columns
int64_t index
int num_columns = self.num_columns
int index

self._check_nullptr()
if not -num_columns <= i < num_columns:
Expand Down
8 changes: 4 additions & 4 deletions python/pyarrow/types.pxi
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -281,12 +281,12 @@ cdef class Schema:
def __len__(self):
return self.schema.num_fields()

def __getitem__(self, int64_t i):
def __getitem__(self, int i):

cdef:
Field result = Field()
int64_t num_fields = self.schema.num_fields()
int64_t index
int num_fields = self.schema.num_fields()
int index

if not -num_fields <= i < num_fields:
raise IndexError(
Expand DownExpand Up@@ -456,7 +456,7 @@ def field(name, DataType type, bint nullable=True, dict metadata=None):
convert_metadata(metadata, &c_meta)

result.sp_field.reset(new CField(tobytes(name), type.sp_type,
nullable, c_meta))
nullable == 1, c_meta))
result.field = result.sp_field.get()
result.type = type
return result
Expand Down
6 changes: 6 additions & 0 deletions python/setup.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -92,6 +92,8 @@ def initialize_options(self):
self.extra_cmake_args = os.environ.get('PYARROW_CMAKE_OPTIONS', '')
self.build_type = os.environ.get('PYARROW_BUILD_TYPE', 'debug').lower()

self.cmake_cxxflags = os.environ.get('PYARROW_CXXFLAGS', '')

if sys.platform == 'win32':
# Cannot do debug builds in Windows unless Python itself is a debug
# build
Expand DownExpand Up@@ -146,6 +148,10 @@ def _run_cmake(self):
if self.with_plasma:
cmake_options.append('-DPYARROW_BUILD_PLASMA=on')

if len(self.cmake_cxxflags) > 0:
cmake_options.append('-DPYARROW_CXXFLAGS="{0}"'
.format(self.cmake_cxxflags))

if self.bundle_arrow_cpp:
cmake_options.append('-DPYARROW_BUNDLE_ARROW_CPP=ON')
# ARROW-1090: work around CMake rough edges
Expand Down
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all 
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '

Skip to content
ARROW-932: [Python] Fix MSVC compiler warnings, build Python with /WX and -Werror in CI#913

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.

 Sign up for GitHub

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

 Closed

wesm
wants to merge
6
commits into

from

 Closed


ARROW-932: [Python] Fix MSVC compiler warnings, build Python with /WX and -Werror in CI
#913
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


6 commits


Select commit
Hold shift + click to select a range
c32ee09

Remove print statement. Disable MSVC 4190 warning


wesm Jul 29, 2017
5740f00

Add PYARROW_CXXFLAGS option, fix MSVC compiler warnings


wesm Jul 29, 2017
2e8f105

typo


wesm Jul 29, 2017
b5a6d9a

Supress another clang warning


wesm Jul 30, 2017
dedcbb9

Fix typo


wesm Jul 30, 2017
9534ae9

Only pass PYARROW_CXXFLAGS when set


wesm Jul 30, 2017

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


3 changes: 3 additions & 0 deletions
ci/msvc-build.bat

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -114,6 +114,9 @@ popd
@rem see PARQUET-1018

pushd python

set PYARROW_CXXFLAGS=/WX
python setup.py build_ext --inplace --with-parquet --bundle-arrow-cpp bdist_wheel || exit /B
py.test pyarrow -v -s --parquet || exit /B

popd

1 change: 1 addition & 0 deletions
ci/travis_script_python.sh

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -23,6 +23,7 @@ source $TRAVIS_BUILD_DIR/ci/travis_env_common.sh
export ARROW_HOME=$ARROW_CPP_INSTALL
export PARQUET_HOME=$TRAVIS_BUILD_DIR/parquet-env
export LD_LIBRARY_PATH=$ARROW_HOME/lib:$PARQUET_HOME/lib:$LD_LIBRARY_PATH
export PYARROW_CXXFLAGS="-Werror"

build_parquet_cpp() {
 export PARQUET_ARROW_VERSION=$(git rev-parse HEAD)
Expand Down

11 changes: 11 additions & 0 deletions
python/CMakeLists.txt

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -57,6 +57,8 @@ if("${CMAKE_SOURCE_DIR}" STREQUAL "${CMAKE_CURRENT_SOURCE_DIR}")
 option(PYARROW_BUNDLE_ARROW_CPP
 "Bundle the Arrow C++ libraries"
 OFF)
 set(PYARROW_CXXFLAGS "" CACHE STRING
 "Compiler flags to append when compiling Arrow")
endif()

find_program(CCACHE_FOUND ccache)
Expand All@@ -75,13 +77,21 @@ include(CompilerInfo)

# Add common flags
set(CMAKE_CXX_FLAGS "${CXX_COMMON_FLAGS} ${CMAKE_CXX_FLAGS}")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${PYARROW_CXXFLAGS}")

if (NOT MSVC)
 # Enable perf and other tools to work properly
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-omit-frame-pointer")

 # Suppress Cython warnings
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-unused-variable")
else()
 # MSVC version of -Wno-return-type-c-linkage
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /wd4190")

 # Cython generates some bitshift expressions that MSVC does not like in
 # __Pyx_PyFloat_DivideObjC
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /wd4293")
endif()

if ("${COMPILER_FAMILY}" STREQUAL "clang")
Expand All@@ -95,6 +105,7 @@ if ("${COMPILER_FAMILY}" STREQUAL "clang")
 # Cython warnings in clang
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-parentheses-equality")
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-constant-logical-operand")
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-missing-declarations")

 # We have public Cython APIs which return C++ types, which are in an extern
 # "C" blog (no symbol mangling) and clang doesn't like this
Expand Down

1 change: 0 additions & 1 deletion
python/doc/source/development.rst

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -267,7 +267,6 @@ Now, we build and install Arrow C++ libraries
 -DCMAKE_INSTALL_PREFIX=%ARROW_HOME% ^
 -DCMAKE_BUILD_TYPE=Release ^
 -DARROW_BUILD_TESTS=off ^
 -DARROW_ZLIB_VENDORED=off ^
 -DARROW_PYTHON=on ..
 cmake --build . --target INSTALL --config Release
 cd ..\..
Expand Down

2 changes: 1 addition & 1 deletion
python/pyarrow/_parquet.pyx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -636,7 +636,7 @@ cdef class ParquetWriter:
 elif row_group_size == 0:
 raise ValueError('Row group size cannot be 0')

 cdef int c_row_group_size = row_group_size
 cdef int64_t c_row_group_size = row_group_size

 with nogil:
 check_status(self.writer.get()
Expand Down

2 changes: 1 addition & 1 deletion
python/pyarrow/includes/libarrow.pxd

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -369,7 +369,7 @@ cdef extern from "arrow/api.h" namespace "arrow" nogil:
 shared_ptr[CTable]* table)

 int num_columns()
int num_rows()
int64_t num_rows()

 c_bool Equals(const CTable& other)

Expand Down

2 changes: 1 addition & 1 deletion
python/pyarrow/io.pxi

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -923,7 +923,7 @@ cdef class _HdfsClient:
 cdef c_string c_path = tobytes(path)
 with nogil:
 check_status(self.client.get()
 .Delete(c_path, recursive))
 .Delete(c_path, recursive == 1))

 def open(self, path, mode='rb', buffer_size=None, replication=None,
 default_block_size=None):
Expand Down

1 change: 0 additions & 1 deletion
python/pyarrow/scalar.pxi

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -169,7 +169,6 @@ cdef class Time64Value(ArrayValue):
 CTime64Type* dtype = <CTime64Type*> ap.type().get()

 cdef int64_t val = ap.Value(self.index)
 print(val)
 if dtype.unit() == TimeUnit_MICRO:
 return (datetime.datetime(1970, 1, 1) +
 datetime.timedelta(microseconds=val)).time()
Expand Down

10 changes: 5 additions & 5 deletions
python/pyarrow/table.pxi

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -286,7 +286,7 @@ cdef int _schema_from_arrays(
 c_string c_name
 vector[shared_ptr[CField]] fields
 shared_ptr[CDataType] type_
int K = len(arrays)
Py_ssize_t K = len(arrays)

 fields.resize(K)

Expand DownExpand Up@@ -733,7 +733,7 @@ cdef class Table:
 vector[shared_ptr[CColumn]] columns
 shared_ptr[CSchema] schema
 shared_ptr[CTable] table
size_t K = len(arrays)
int i, K = <int> len(arrays)

 _schema_from_arrays(arrays, names, metadata, &schema)

Expand DownExpand Up@@ -841,7 +841,7 @@ cdef class Table:
 self._check_nullptr()
 return pyarrow_wrap_schema(self.table.schema())

 def column(self, int64_t i):
 def column(self, int i):
 """
 Select a column by its numeric index.

Expand All@@ -855,8 +855,8 @@ cdef class Table:
 """
 cdef:
 Column column = Column()
int64_t num_columns = self.num_columns
int64_t index
int num_columns = self.num_columns
int index

 self._check_nullptr()
 if not -num_columns <= i < num_columns:
Expand Down

8 changes: 4 additions & 4 deletions
python/pyarrow/types.pxi

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -281,12 +281,12 @@ cdef class Schema:
 def __len__(self):
 return self.schema.num_fields()

 def __getitem__(self, int64_t i):
 def __getitem__(self, int i):

 cdef:
 Field result = Field()
int64_t num_fields = self.schema.num_fields()
int64_t index
int num_fields = self.schema.num_fields()
int index

 if not -num_fields <= i < num_fields:
 raise IndexError(
Expand DownExpand Up@@ -456,7 +456,7 @@ def field(name, DataType type, bint nullable=True, dict metadata=None):
 convert_metadata(metadata, &c_meta)

 result.sp_field.reset(new CField(tobytes(name), type.sp_type,
 nullable, c_meta))
 nullable == 1, c_meta))
 result.field = result.sp_field.get()
 result.type = type
 return result
Expand Down

6 changes: 6 additions & 0 deletions
python/setup.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -92,6 +92,8 @@ def initialize_options(self):
 self.extra_cmake_args = os.environ.get('PYARROW_CMAKE_OPTIONS', '')
 self.build_type = os.environ.get('PYARROW_BUILD_TYPE', 'debug').lower()

 self.cmake_cxxflags = os.environ.get('PYARROW_CXXFLAGS', '')

 if sys.platform == 'win32':
 # Cannot do debug builds in Windows unless Python itself is a debug
 # build
Expand DownExpand Up@@ -146,6 +148,10 @@ def _run_cmake(self):
 if self.with_plasma:
 cmake_options.append('-DPYARROW_BUILD_PLASMA=on')

 if len(self.cmake_cxxflags) > 0:
 cmake_options.append('-DPYARROW_CXXFLAGS="{0}"'
 .format(self.cmake_cxxflags))

 if self.bundle_arrow_cpp:
 cmake_options.append('-DPYARROW_BUNDLE_ARROW_CPP=ON')
 # ARROW-1090: work around CMake rough edges
Expand Down

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix");
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
ARROW-932: [Python] Fix MSVC compiler warnings, build Python with /WX and -Werror in CI#913

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.

 Sign up for GitHub

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

 Closed

wesm
wants to merge
6
commits into

from

 Closed


ARROW-932: [Python] Fix MSVC compiler warnings, build Python with /WX and -Werror in CI
#913
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


6 commits


Select commit
Hold shift + click to select a range
c32ee09

Remove print statement. Disable MSVC 4190 warning


wesm Jul 29, 2017
5740f00

Add PYARROW_CXXFLAGS option, fix MSVC compiler warnings


wesm Jul 29, 2017
2e8f105

typo


wesm Jul 29, 2017
b5a6d9a

Supress another clang warning


wesm Jul 30, 2017
dedcbb9

Fix typo


wesm Jul 30, 2017
9534ae9

Only pass PYARROW_CXXFLAGS when set


wesm Jul 30, 2017

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


3 changes: 3 additions & 0 deletions
ci/msvc-build.bat

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -114,6 +114,9 @@ popd
@rem see PARQUET-1018

pushd python

set PYARROW_CXXFLAGS=/WX
python setup.py build_ext --inplace --with-parquet --bundle-arrow-cpp bdist_wheel || exit /B
py.test pyarrow -v -s --parquet || exit /B

popd

1 change: 1 addition & 0 deletions
ci/travis_script_python.sh

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -23,6 +23,7 @@ source $TRAVIS_BUILD_DIR/ci/travis_env_common.sh
export ARROW_HOME=$ARROW_CPP_INSTALL
export PARQUET_HOME=$TRAVIS_BUILD_DIR/parquet-env
export LD_LIBRARY_PATH=$ARROW_HOME/lib:$PARQUET_HOME/lib:$LD_LIBRARY_PATH
export PYARROW_CXXFLAGS="-Werror"

build_parquet_cpp() {
 export PARQUET_ARROW_VERSION=$(git rev-parse HEAD)
Expand Down

11 changes: 11 additions & 0 deletions
python/CMakeLists.txt

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -57,6 +57,8 @@ if("${CMAKE_SOURCE_DIR}" STREQUAL "${CMAKE_CURRENT_SOURCE_DIR}")
 option(PYARROW_BUNDLE_ARROW_CPP
 "Bundle the Arrow C++ libraries"
 OFF)
 set(PYARROW_CXXFLAGS "" CACHE STRING
 "Compiler flags to append when compiling Arrow")
endif()

find_program(CCACHE_FOUND ccache)
Expand All@@ -75,13 +77,21 @@ include(CompilerInfo)

# Add common flags
set(CMAKE_CXX_FLAGS "${CXX_COMMON_FLAGS} ${CMAKE_CXX_FLAGS}")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${PYARROW_CXXFLAGS}")

if (NOT MSVC)
 # Enable perf and other tools to work properly
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-omit-frame-pointer")

 # Suppress Cython warnings
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-unused-variable")
else()
 # MSVC version of -Wno-return-type-c-linkage
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /wd4190")

 # Cython generates some bitshift expressions that MSVC does not like in
 # __Pyx_PyFloat_DivideObjC
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /wd4293")
endif()

if ("${COMPILER_FAMILY}" STREQUAL "clang")
Expand All@@ -95,6 +105,7 @@ if ("${COMPILER_FAMILY}" STREQUAL "clang")
 # Cython warnings in clang
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-parentheses-equality")
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-constant-logical-operand")
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-missing-declarations")

 # We have public Cython APIs which return C++ types, which are in an extern
 # "C" blog (no symbol mangling) and clang doesn't like this
Expand Down

1 change: 0 additions & 1 deletion
python/doc/source/development.rst

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -267,7 +267,6 @@ Now, we build and install Arrow C++ libraries
 -DCMAKE_INSTALL_PREFIX=%ARROW_HOME% ^
 -DCMAKE_BUILD_TYPE=Release ^
 -DARROW_BUILD_TESTS=off ^
 -DARROW_ZLIB_VENDORED=off ^
 -DARROW_PYTHON=on ..
 cmake --build . --target INSTALL --config Release
 cd ..\..
Expand Down

2 changes: 1 addition & 1 deletion
python/pyarrow/_parquet.pyx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -636,7 +636,7 @@ cdef class ParquetWriter:
 elif row_group_size == 0:
 raise ValueError('Row group size cannot be 0')

 cdef int c_row_group_size = row_group_size
 cdef int64_t c_row_group_size = row_group_size

 with nogil:
 check_status(self.writer.get()
Expand Down

2 changes: 1 addition & 1 deletion
python/pyarrow/includes/libarrow.pxd

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -369,7 +369,7 @@ cdef extern from "arrow/api.h" namespace "arrow" nogil:
 shared_ptr[CTable]* table)

 int num_columns()
int num_rows()
int64_t num_rows()

 c_bool Equals(const CTable& other)

Expand Down

2 changes: 1 addition & 1 deletion
python/pyarrow/io.pxi

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -923,7 +923,7 @@ cdef class _HdfsClient:
 cdef c_string c_path = tobytes(path)
 with nogil:
 check_status(self.client.get()
 .Delete(c_path, recursive))
 .Delete(c_path, recursive == 1))

 def open(self, path, mode='rb', buffer_size=None, replication=None,
 default_block_size=None):
Expand Down

1 change: 0 additions & 1 deletion
python/pyarrow/scalar.pxi

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -169,7 +169,6 @@ cdef class Time64Value(ArrayValue):
 CTime64Type* dtype = <CTime64Type*> ap.type().get()

 cdef int64_t val = ap.Value(self.index)
 print(val)
 if dtype.unit() == TimeUnit_MICRO:
 return (datetime.datetime(1970, 1, 1) +
 datetime.timedelta(microseconds=val)).time()
Expand Down

10 changes: 5 additions & 5 deletions
python/pyarrow/table.pxi

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -286,7 +286,7 @@ cdef int _schema_from_arrays(
 c_string c_name
 vector[shared_ptr[CField]] fields
 shared_ptr[CDataType] type_
int K = len(arrays)
Py_ssize_t K = len(arrays)

 fields.resize(K)

Expand DownExpand Up@@ -733,7 +733,7 @@ cdef class Table:
 vector[shared_ptr[CColumn]] columns
 shared_ptr[CSchema] schema
 shared_ptr[CTable] table
size_t K = len(arrays)
int i, K = <int> len(arrays)

 _schema_from_arrays(arrays, names, metadata, &schema)

Expand DownExpand Up@@ -841,7 +841,7 @@ cdef class Table:
 self._check_nullptr()
 return pyarrow_wrap_schema(self.table.schema())

 def column(self, int64_t i):
 def column(self, int i):
 """
 Select a column by its numeric index.

Expand All@@ -855,8 +855,8 @@ cdef class Table:
 """
 cdef:
 Column column = Column()
int64_t num_columns = self.num_columns
int64_t index
int num_columns = self.num_columns
int index

 self._check_nullptr()
 if not -num_columns <= i < num_columns:
Expand Down

8 changes: 4 additions & 4 deletions
python/pyarrow/types.pxi

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -281,12 +281,12 @@ cdef class Schema:
 def __len__(self):
 return self.schema.num_fields()

 def __getitem__(self, int64_t i):
 def __getitem__(self, int i):

 cdef:
 Field result = Field()
int64_t num_fields = self.schema.num_fields()
int64_t index
int num_fields = self.schema.num_fields()
int index

 if not -num_fields <= i < num_fields:
 raise IndexError(
Expand DownExpand Up@@ -456,7 +456,7 @@ def field(name, DataType type, bint nullable=True, dict metadata=None):
 convert_metadata(metadata, &c_meta)

 result.sp_field.reset(new CField(tobytes(name), type.sp_type,
 nullable, c_meta))
 nullable == 1, c_meta))
 result.field = result.sp_field.get()
 result.type = type
 return result
Expand Down

6 changes: 6 additions & 0 deletions
python/setup.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -92,6 +92,8 @@ def initialize_options(self):
 self.extra_cmake_args = os.environ.get('PYARROW_CMAKE_OPTIONS', '')
 self.build_type = os.environ.get('PYARROW_BUILD_TYPE', 'debug').lower()

 self.cmake_cxxflags = os.environ.get('PYARROW_CXXFLAGS', '')

 if sys.platform == 'win32':
 # Cannot do debug builds in Windows unless Python itself is a debug
 # build
Expand DownExpand Up@@ -146,6 +148,10 @@ def _run_cmake(self):
 if self.with_plasma:
 cmake_options.append('-DPYARROW_BUILD_PLASMA=on')

 if len(self.cmake_cxxflags) > 0:
 cmake_options.append('-DPYARROW_CXXFLAGS="{0}"'
 .format(self.cmake_cxxflags))

 if self.bundle_arrow_cpp:
 cmake_options.append('-DPYARROW_BUNDLE_ARROW_CPP=ON')
 # ARROW-1090: work around CMake rough edges
Expand Down

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms");
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
ARROW-932: [Python] Fix MSVC compiler warnings, build Python with /WX and -Werror in CI#913

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.

 Sign up for GitHub

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

 Closed

wesm
wants to merge
6
commits into

from

 Closed


ARROW-932: [Python] Fix MSVC compiler warnings, build Python with /WX and -Werror in CI
#913
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


6 commits


Select commit
Hold shift + click to select a range
c32ee09

Remove print statement. Disable MSVC 4190 warning


wesm Jul 29, 2017
5740f00

Add PYARROW_CXXFLAGS option, fix MSVC compiler warnings


wesm Jul 29, 2017
2e8f105

typo


wesm Jul 29, 2017
b5a6d9a

Supress another clang warning


wesm Jul 30, 2017
dedcbb9

Fix typo


wesm Jul 30, 2017
9534ae9

Only pass PYARROW_CXXFLAGS when set


wesm Jul 30, 2017

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


3 changes: 3 additions & 0 deletions
ci/msvc-build.bat

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -114,6 +114,9 @@ popd
@rem see PARQUET-1018

pushd python

set PYARROW_CXXFLAGS=/WX
python setup.py build_ext --inplace --with-parquet --bundle-arrow-cpp bdist_wheel || exit /B
py.test pyarrow -v -s --parquet || exit /B

popd

1 change: 1 addition & 0 deletions
ci/travis_script_python.sh

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -23,6 +23,7 @@ source $TRAVIS_BUILD_DIR/ci/travis_env_common.sh
export ARROW_HOME=$ARROW_CPP_INSTALL
export PARQUET_HOME=$TRAVIS_BUILD_DIR/parquet-env
export LD_LIBRARY_PATH=$ARROW_HOME/lib:$PARQUET_HOME/lib:$LD_LIBRARY_PATH
export PYARROW_CXXFLAGS="-Werror"

build_parquet_cpp() {
 export PARQUET_ARROW_VERSION=$(git rev-parse HEAD)
Expand Down

11 changes: 11 additions & 0 deletions
python/CMakeLists.txt

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -57,6 +57,8 @@ if("${CMAKE_SOURCE_DIR}" STREQUAL "${CMAKE_CURRENT_SOURCE_DIR}")
 option(PYARROW_BUNDLE_ARROW_CPP
 "Bundle the Arrow C++ libraries"
 OFF)
 set(PYARROW_CXXFLAGS "" CACHE STRING
 "Compiler flags to append when compiling Arrow")
endif()

find_program(CCACHE_FOUND ccache)
Expand All@@ -75,13 +77,21 @@ include(CompilerInfo)

# Add common flags
set(CMAKE_CXX_FLAGS "${CXX_COMMON_FLAGS} ${CMAKE_CXX_FLAGS}")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${PYARROW_CXXFLAGS}")

if (NOT MSVC)
 # Enable perf and other tools to work properly
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-omit-frame-pointer")

 # Suppress Cython warnings
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-unused-variable")
else()
 # MSVC version of -Wno-return-type-c-linkage
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /wd4190")

 # Cython generates some bitshift expressions that MSVC does not like in
 # __Pyx_PyFloat_DivideObjC
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /wd4293")
endif()

if ("${COMPILER_FAMILY}" STREQUAL "clang")
Expand All@@ -95,6 +105,7 @@ if ("${COMPILER_FAMILY}" STREQUAL "clang")
 # Cython warnings in clang
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-parentheses-equality")
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-constant-logical-operand")
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-missing-declarations")

 # We have public Cython APIs which return C++ types, which are in an extern
 # "C" blog (no symbol mangling) and clang doesn't like this
Expand Down

1 change: 0 additions & 1 deletion
python/doc/source/development.rst

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -267,7 +267,6 @@ Now, we build and install Arrow C++ libraries
 -DCMAKE_INSTALL_PREFIX=%ARROW_HOME% ^
 -DCMAKE_BUILD_TYPE=Release ^
 -DARROW_BUILD_TESTS=off ^
 -DARROW_ZLIB_VENDORED=off ^
 -DARROW_PYTHON=on ..
 cmake --build . --target INSTALL --config Release
 cd ..\..
Expand Down

2 changes: 1 addition & 1 deletion
python/pyarrow/_parquet.pyx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -636,7 +636,7 @@ cdef class ParquetWriter:
 elif row_group_size == 0:
 raise ValueError('Row group size cannot be 0')

 cdef int c_row_group_size = row_group_size
 cdef int64_t c_row_group_size = row_group_size

 with nogil:
 check_status(self.writer.get()
Expand Down

2 changes: 1 addition & 1 deletion
python/pyarrow/includes/libarrow.pxd

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -369,7 +369,7 @@ cdef extern from "arrow/api.h" namespace "arrow" nogil:
 shared_ptr[CTable]* table)

 int num_columns()
int num_rows()
int64_t num_rows()

 c_bool Equals(const CTable& other)

Expand Down

2 changes: 1 addition & 1 deletion
python/pyarrow/io.pxi

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -923,7 +923,7 @@ cdef class _HdfsClient:
 cdef c_string c_path = tobytes(path)
 with nogil:
 check_status(self.client.get()
 .Delete(c_path, recursive))
 .Delete(c_path, recursive == 1))

 def open(self, path, mode='rb', buffer_size=None, replication=None,
 default_block_size=None):
Expand Down

1 change: 0 additions & 1 deletion
python/pyarrow/scalar.pxi

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -169,7 +169,6 @@ cdef class Time64Value(ArrayValue):
 CTime64Type* dtype = <CTime64Type*> ap.type().get()

 cdef int64_t val = ap.Value(self.index)
 print(val)
 if dtype.unit() == TimeUnit_MICRO:
 return (datetime.datetime(1970, 1, 1) +
 datetime.timedelta(microseconds=val)).time()
Expand Down

10 changes: 5 additions & 5 deletions
python/pyarrow/table.pxi

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -286,7 +286,7 @@ cdef int _schema_from_arrays(
 c_string c_name
 vector[shared_ptr[CField]] fields
 shared_ptr[CDataType] type_
int K = len(arrays)
Py_ssize_t K = len(arrays)

 fields.resize(K)

Expand DownExpand Up@@ -733,7 +733,7 @@ cdef class Table:
 vector[shared_ptr[CColumn]] columns
 shared_ptr[CSchema] schema
 shared_ptr[CTable] table
size_t K = len(arrays)
int i, K = <int> len(arrays)

 _schema_from_arrays(arrays, names, metadata, &schema)

Expand DownExpand Up@@ -841,7 +841,7 @@ cdef class Table:
 self._check_nullptr()
 return pyarrow_wrap_schema(self.table.schema())

 def column(self, int64_t i):
 def column(self, int i):
 """
 Select a column by its numeric index.

Expand All@@ -855,8 +855,8 @@ cdef class Table:
 """
 cdef:
 Column column = Column()
int64_t num_columns = self.num_columns
int64_t index
int num_columns = self.num_columns
int index

 self._check_nullptr()
 if not -num_columns <= i < num_columns:
Expand Down

8 changes: 4 additions & 4 deletions
python/pyarrow/types.pxi

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -281,12 +281,12 @@ cdef class Schema:
 def __len__(self):
 return self.schema.num_fields()

 def __getitem__(self, int64_t i):
 def __getitem__(self, int i):

 cdef:
 Field result = Field()
int64_t num_fields = self.schema.num_fields()
int64_t index
int num_fields = self.schema.num_fields()
int index

 if not -num_fields <= i < num_fields:
 raise IndexError(
Expand DownExpand Up@@ -456,7 +456,7 @@ def field(name, DataType type, bint nullable=True, dict metadata=None):
 convert_metadata(metadata, &c_meta)

 result.sp_field.reset(new CField(tobytes(name), type.sp_type,
 nullable, c_meta))
 nullable == 1, c_meta))
 result.field = result.sp_field.get()
 result.type = type
 return result
Expand Down

6 changes: 6 additions & 0 deletions
python/setup.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -92,6 +92,8 @@ def initialize_options(self):
 self.extra_cmake_args = os.environ.get('PYARROW_CMAKE_OPTIONS', '')
 self.build_type = os.environ.get('PYARROW_BUILD_TYPE', 'debug').lower()

 self.cmake_cxxflags = os.environ.get('PYARROW_CXXFLAGS', '')

 if sys.platform == 'win32':
 # Cannot do debug builds in Windows unless Python itself is a debug
 # build
Expand DownExpand Up@@ -146,6 +148,10 @@ def _run_cmake(self):
 if self.with_plasma:
 cmake_options.append('-DPYARROW_BUILD_PLASMA=on')

 if len(self.cmake_cxxflags) > 0:
 cmake_options.append('-DPYARROW_CXXFLAGS="{0}"'
 .format(self.cmake_cxxflags))

 if self.bundle_arrow_cpp:
 cmake_options.append('-DPYARROW_BUNDLE_ARROW_CPP=ON')
 # ARROW-1090: work around CMake rough edges
Expand Down

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links");
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '

Skip to content
ARROW-932: [Python] Fix MSVC compiler warnings, build Python with /WX and -Werror in CI#913

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.

 Sign up for GitHub

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

 Closed

wesm
wants to merge
6
commits into

from

 Closed


ARROW-932: [Python] Fix MSVC compiler warnings, build Python with /WX and -Werror in CI
#913
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


6 commits


Select commit
Hold shift + click to select a range
c32ee09

Remove print statement. Disable MSVC 4190 warning


wesm Jul 29, 2017
5740f00

Add PYARROW_CXXFLAGS option, fix MSVC compiler warnings


wesm Jul 29, 2017
2e8f105

typo


wesm Jul 29, 2017
b5a6d9a

Supress another clang warning


wesm Jul 30, 2017
dedcbb9

Fix typo


wesm Jul 30, 2017
9534ae9

Only pass PYARROW_CXXFLAGS when set


wesm Jul 30, 2017

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


3 changes: 3 additions & 0 deletions
ci/msvc-build.bat

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -114,6 +114,9 @@ popd
@rem see PARQUET-1018

pushd python

set PYARROW_CXXFLAGS=/WX
python setup.py build_ext --inplace --with-parquet --bundle-arrow-cpp bdist_wheel || exit /B
py.test pyarrow -v -s --parquet || exit /B

popd

1 change: 1 addition & 0 deletions
ci/travis_script_python.sh

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -23,6 +23,7 @@ source $TRAVIS_BUILD_DIR/ci/travis_env_common.sh
export ARROW_HOME=$ARROW_CPP_INSTALL
export PARQUET_HOME=$TRAVIS_BUILD_DIR/parquet-env
export LD_LIBRARY_PATH=$ARROW_HOME/lib:$PARQUET_HOME/lib:$LD_LIBRARY_PATH
export PYARROW_CXXFLAGS="-Werror"

build_parquet_cpp() {
 export PARQUET_ARROW_VERSION=$(git rev-parse HEAD)
Expand Down

11 changes: 11 additions & 0 deletions
python/CMakeLists.txt

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -57,6 +57,8 @@ if("${CMAKE_SOURCE_DIR}" STREQUAL "${CMAKE_CURRENT_SOURCE_DIR}")
 option(PYARROW_BUNDLE_ARROW_CPP
 "Bundle the Arrow C++ libraries"
 OFF)
 set(PYARROW_CXXFLAGS "" CACHE STRING
 "Compiler flags to append when compiling Arrow")
endif()

find_program(CCACHE_FOUND ccache)
Expand All@@ -75,13 +77,21 @@ include(CompilerInfo)

# Add common flags
set(CMAKE_CXX_FLAGS "${CXX_COMMON_FLAGS} ${CMAKE_CXX_FLAGS}")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${PYARROW_CXXFLAGS}")

if (NOT MSVC)
 # Enable perf and other tools to work properly
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-omit-frame-pointer")

 # Suppress Cython warnings
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-unused-variable")
else()
 # MSVC version of -Wno-return-type-c-linkage
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /wd4190")

 # Cython generates some bitshift expressions that MSVC does not like in
 # __Pyx_PyFloat_DivideObjC
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /wd4293")
endif()

if ("${COMPILER_FAMILY}" STREQUAL "clang")
Expand All@@ -95,6 +105,7 @@ if ("${COMPILER_FAMILY}" STREQUAL "clang")
 # Cython warnings in clang
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-parentheses-equality")
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-constant-logical-operand")
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-missing-declarations")

 # We have public Cython APIs which return C++ types, which are in an extern
 # "C" blog (no symbol mangling) and clang doesn't like this
Expand Down

1 change: 0 additions & 1 deletion
python/doc/source/development.rst

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -267,7 +267,6 @@ Now, we build and install Arrow C++ libraries
 -DCMAKE_INSTALL_PREFIX=%ARROW_HOME% ^
 -DCMAKE_BUILD_TYPE=Release ^
 -DARROW_BUILD_TESTS=off ^
 -DARROW_ZLIB_VENDORED=off ^
 -DARROW_PYTHON=on ..
 cmake --build . --target INSTALL --config Release
 cd ..\..
Expand Down

2 changes: 1 addition & 1 deletion
python/pyarrow/_parquet.pyx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -636,7 +636,7 @@ cdef class ParquetWriter:
 elif row_group_size == 0:
 raise ValueError('Row group size cannot be 0')

 cdef int c_row_group_size = row_group_size
 cdef int64_t c_row_group_size = row_group_size

 with nogil:
 check_status(self.writer.get()
Expand Down

2 changes: 1 addition & 1 deletion
python/pyarrow/includes/libarrow.pxd

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -369,7 +369,7 @@ cdef extern from "arrow/api.h" namespace "arrow" nogil:
 shared_ptr[CTable]* table)

 int num_columns()
int num_rows()
int64_t num_rows()

 c_bool Equals(const CTable& other)

Expand Down

2 changes: 1 addition & 1 deletion
python/pyarrow/io.pxi

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -923,7 +923,7 @@ cdef class _HdfsClient:
 cdef c_string c_path = tobytes(path)
 with nogil:
 check_status(self.client.get()
 .Delete(c_path, recursive))
 .Delete(c_path, recursive == 1))

 def open(self, path, mode='rb', buffer_size=None, replication=None,
 default_block_size=None):
Expand Down

1 change: 0 additions & 1 deletion
python/pyarrow/scalar.pxi

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -169,7 +169,6 @@ cdef class Time64Value(ArrayValue):
 CTime64Type* dtype = <CTime64Type*> ap.type().get()

 cdef int64_t val = ap.Value(self.index)
 print(val)
 if dtype.unit() == TimeUnit_MICRO:
 return (datetime.datetime(1970, 1, 1) +
 datetime.timedelta(microseconds=val)).time()
Expand Down

10 changes: 5 additions & 5 deletions
python/pyarrow/table.pxi

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -286,7 +286,7 @@ cdef int _schema_from_arrays(
 c_string c_name
 vector[shared_ptr[CField]] fields
 shared_ptr[CDataType] type_
int K = len(arrays)
Py_ssize_t K = len(arrays)

 fields.resize(K)

Expand DownExpand Up@@ -733,7 +733,7 @@ cdef class Table:
 vector[shared_ptr[CColumn]] columns
 shared_ptr[CSchema] schema
 shared_ptr[CTable] table
size_t K = len(arrays)
int i, K = <int> len(arrays)

 _schema_from_arrays(arrays, names, metadata, &schema)

Expand DownExpand Up@@ -841,7 +841,7 @@ cdef class Table:
 self._check_nullptr()
 return pyarrow_wrap_schema(self.table.schema())

 def column(self, int64_t i):
 def column(self, int i):
 """
 Select a column by its numeric index.

Expand All@@ -855,8 +855,8 @@ cdef class Table:
 """
 cdef:
 Column column = Column()
int64_t num_columns = self.num_columns
int64_t index
int num_columns = self.num_columns
int index

 self._check_nullptr()
 if not -num_columns <= i < num_columns:
Expand Down

8 changes: 4 additions & 4 deletions
python/pyarrow/types.pxi

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -281,12 +281,12 @@ cdef class Schema:
 def __len__(self):
 return self.schema.num_fields()

 def __getitem__(self, int64_t i):
 def __getitem__(self, int i):

 cdef:
 Field result = Field()
int64_t num_fields = self.schema.num_fields()
int64_t index
int num_fields = self.schema.num_fields()
int index

 if not -num_fields <= i < num_fields:
 raise IndexError(
Expand DownExpand Up@@ -456,7 +456,7 @@ def field(name, DataType type, bint nullable=True, dict metadata=None):
 convert_metadata(metadata, &c_meta)

 result.sp_field.reset(new CField(tobytes(name), type.sp_type,
 nullable, c_meta))
 nullable == 1, c_meta))
 result.field = result.sp_field.get()
 result.type = type
 return result
Expand Down

6 changes: 6 additions & 0 deletions
python/setup.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -92,6 +92,8 @@ def initialize_options(self):
 self.extra_cmake_args = os.environ.get('PYARROW_CMAKE_OPTIONS', '')
 self.build_type = os.environ.get('PYARROW_BUILD_TYPE', 'debug').lower()

 self.cmake_cxxflags = os.environ.get('PYARROW_CXXFLAGS', '')

 if sys.platform == 'win32':
 # Cannot do debug builds in Windows unless Python itself is a debug
 # build
Expand DownExpand Up@@ -146,6 +148,10 @@ def _run_cmake(self):
 if self.with_plasma:
 cmake_options.append('-DPYARROW_BUILD_PLASMA=on')

 if len(self.cmake_cxxflags) > 0:
 cmake_options.append('-DPYARROW_CXXFLAGS="{0}"'
 .format(self.cmake_cxxflags))

 if self.bundle_arrow_cpp:
 cmake_options.append('-DPYARROW_BUNDLE_ARROW_CPP=ON')
 # ARROW-1090: work around CMake rough edges
Expand Down

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default");
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
ARROW-932: [Python] Fix MSVC compiler warnings, build Python with /WX and -Werror in CI#913

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.

 Sign up for GitHub

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

 Closed

wesm
wants to merge
6
commits into

from

 Closed


ARROW-932: [Python] Fix MSVC compiler warnings, build Python with /WX and -Werror in CI
#913
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


6 commits


Select commit
Hold shift + click to select a range
c32ee09

Remove print statement. Disable MSVC 4190 warning


wesm Jul 29, 2017
5740f00

Add PYARROW_CXXFLAGS option, fix MSVC compiler warnings


wesm Jul 29, 2017
2e8f105

typo


wesm Jul 29, 2017
b5a6d9a

Supress another clang warning


wesm Jul 30, 2017
dedcbb9

Fix typo


wesm Jul 30, 2017
9534ae9

Only pass PYARROW_CXXFLAGS when set


wesm Jul 30, 2017

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


3 changes: 3 additions & 0 deletions
ci/msvc-build.bat

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -114,6 +114,9 @@ popd
@rem see PARQUET-1018

pushd python

set PYARROW_CXXFLAGS=/WX
python setup.py build_ext --inplace --with-parquet --bundle-arrow-cpp bdist_wheel || exit /B
py.test pyarrow -v -s --parquet || exit /B

popd

1 change: 1 addition & 0 deletions
ci/travis_script_python.sh

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -23,6 +23,7 @@ source $TRAVIS_BUILD_DIR/ci/travis_env_common.sh
export ARROW_HOME=$ARROW_CPP_INSTALL
export PARQUET_HOME=$TRAVIS_BUILD_DIR/parquet-env
export LD_LIBRARY_PATH=$ARROW_HOME/lib:$PARQUET_HOME/lib:$LD_LIBRARY_PATH
export PYARROW_CXXFLAGS="-Werror"

build_parquet_cpp() {
 export PARQUET_ARROW_VERSION=$(git rev-parse HEAD)
Expand Down

11 changes: 11 additions & 0 deletions
python/CMakeLists.txt

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -57,6 +57,8 @@ if("${CMAKE_SOURCE_DIR}" STREQUAL "${CMAKE_CURRENT_SOURCE_DIR}")
 option(PYARROW_BUNDLE_ARROW_CPP
 "Bundle the Arrow C++ libraries"
 OFF)
 set(PYARROW_CXXFLAGS "" CACHE STRING
 "Compiler flags to append when compiling Arrow")
endif()

find_program(CCACHE_FOUND ccache)
Expand All@@ -75,13 +77,21 @@ include(CompilerInfo)

# Add common flags
set(CMAKE_CXX_FLAGS "${CXX_COMMON_FLAGS} ${CMAKE_CXX_FLAGS}")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${PYARROW_CXXFLAGS}")

if (NOT MSVC)
 # Enable perf and other tools to work properly
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-omit-frame-pointer")

 # Suppress Cython warnings
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-unused-variable")
else()
 # MSVC version of -Wno-return-type-c-linkage
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /wd4190")

 # Cython generates some bitshift expressions that MSVC does not like in
 # __Pyx_PyFloat_DivideObjC
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /wd4293")
endif()

if ("${COMPILER_FAMILY}" STREQUAL "clang")
Expand All@@ -95,6 +105,7 @@ if ("${COMPILER_FAMILY}" STREQUAL "clang")
 # Cython warnings in clang
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-parentheses-equality")
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-constant-logical-operand")
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-missing-declarations")

 # We have public Cython APIs which return C++ types, which are in an extern
 # "C" blog (no symbol mangling) and clang doesn't like this
Expand Down

1 change: 0 additions & 1 deletion
python/doc/source/development.rst

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -267,7 +267,6 @@ Now, we build and install Arrow C++ libraries
 -DCMAKE_INSTALL_PREFIX=%ARROW_HOME% ^
 -DCMAKE_BUILD_TYPE=Release ^
 -DARROW_BUILD_TESTS=off ^
 -DARROW_ZLIB_VENDORED=off ^
 -DARROW_PYTHON=on ..
 cmake --build . --target INSTALL --config Release
 cd ..\..
Expand Down

2 changes: 1 addition & 1 deletion
python/pyarrow/_parquet.pyx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -636,7 +636,7 @@ cdef class ParquetWriter:
 elif row_group_size == 0:
 raise ValueError('Row group size cannot be 0')

 cdef int c_row_group_size = row_group_size
 cdef int64_t c_row_group_size = row_group_size

 with nogil:
 check_status(self.writer.get()
Expand Down

2 changes: 1 addition & 1 deletion
python/pyarrow/includes/libarrow.pxd

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -369,7 +369,7 @@ cdef extern from "arrow/api.h" namespace "arrow" nogil:
 shared_ptr[CTable]* table)

 int num_columns()
int num_rows()
int64_t num_rows()

 c_bool Equals(const CTable& other)

Expand Down

2 changes: 1 addition & 1 deletion
python/pyarrow/io.pxi

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -923,7 +923,7 @@ cdef class _HdfsClient:
 cdef c_string c_path = tobytes(path)
 with nogil:
 check_status(self.client.get()
 .Delete(c_path, recursive))
 .Delete(c_path, recursive == 1))

 def open(self, path, mode='rb', buffer_size=None, replication=None,
 default_block_size=None):
Expand Down

1 change: 0 additions & 1 deletion
python/pyarrow/scalar.pxi

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -169,7 +169,6 @@ cdef class Time64Value(ArrayValue):
 CTime64Type* dtype = <CTime64Type*> ap.type().get()

 cdef int64_t val = ap.Value(self.index)
 print(val)
 if dtype.unit() == TimeUnit_MICRO:
 return (datetime.datetime(1970, 1, 1) +
 datetime.timedelta(microseconds=val)).time()
Expand Down

10 changes: 5 additions & 5 deletions
python/pyarrow/table.pxi

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -286,7 +286,7 @@ cdef int _schema_from_arrays(
 c_string c_name
 vector[shared_ptr[CField]] fields
 shared_ptr[CDataType] type_
int K = len(arrays)
Py_ssize_t K = len(arrays)

 fields.resize(K)

Expand DownExpand Up@@ -733,7 +733,7 @@ cdef class Table:
 vector[shared_ptr[CColumn]] columns
 shared_ptr[CSchema] schema
 shared_ptr[CTable] table
size_t K = len(arrays)
int i, K = <int> len(arrays)

 _schema_from_arrays(arrays, names, metadata, &schema)

Expand DownExpand Up@@ -841,7 +841,7 @@ cdef class Table:
 self._check_nullptr()
 return pyarrow_wrap_schema(self.table.schema())

 def column(self, int64_t i):
 def column(self, int i):
 """
 Select a column by its numeric index.

Expand All@@ -855,8 +855,8 @@ cdef class Table:
 """
 cdef:
 Column column = Column()
int64_t num_columns = self.num_columns
int64_t index
int num_columns = self.num_columns
int index

 self._check_nullptr()
 if not -num_columns <= i < num_columns:
Expand Down

8 changes: 4 additions & 4 deletions
python/pyarrow/types.pxi

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -281,12 +281,12 @@ cdef class Schema:
 def __len__(self):
 return self.schema.num_fields()

 def __getitem__(self, int64_t i):
 def __getitem__(self, int i):

 cdef:
 Field result = Field()
int64_t num_fields = self.schema.num_fields()
int64_t index
int num_fields = self.schema.num_fields()
int index

 if not -num_fields <= i < num_fields:
 raise IndexError(
Expand DownExpand Up@@ -456,7 +456,7 @@ def field(name, DataType type, bint nullable=True, dict metadata=None):
 convert_metadata(metadata, &c_meta)

 result.sp_field.reset(new CField(tobytes(name), type.sp_type,
 nullable, c_meta))
 nullable == 1, c_meta))
 result.field = result.sp_field.get()
 result.type = type
 return result
Expand Down

6 changes: 6 additions & 0 deletions
python/setup.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -92,6 +92,8 @@ def initialize_options(self):
 self.extra_cmake_args = os.environ.get('PYARROW_CMAKE_OPTIONS', '')
 self.build_type = os.environ.get('PYARROW_BUILD_TYPE', 'debug').lower()

 self.cmake_cxxflags = os.environ.get('PYARROW_CXXFLAGS', '')

 if sys.platform == 'win32':
 # Cannot do debug builds in Windows unless Python itself is a debug
 # build
Expand DownExpand Up@@ -146,6 +148,10 @@ def _run_cmake(self):
 if self.with_plasma:
 cmake_options.append('-DPYARROW_BUILD_PLASMA=on')

 if len(self.cmake_cxxflags) > 0:
 cmake_options.append('-DPYARROW_CXXFLAGS="{0}"'
 .format(self.cmake_cxxflags))

 if self.bundle_arrow_cpp:
 cmake_options.append('-DPYARROW_BUNDLE_ARROW_CPP=ON')
 # ARROW-1090: work around CMake rough edges
Expand Down

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers");
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
ARROW-932: [Python] Fix MSVC compiler warnings, build Python with /WX and -Werror in CI#913

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.

 Sign up for GitHub

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

 Closed

wesm
wants to merge
6
commits into

from

 Closed


ARROW-932: [Python] Fix MSVC compiler warnings, build Python with /WX and -Werror in CI
#913
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


6 commits


Select commit
Hold shift + click to select a range
c32ee09

Remove print statement. Disable MSVC 4190 warning


wesm Jul 29, 2017
5740f00

Add PYARROW_CXXFLAGS option, fix MSVC compiler warnings


wesm Jul 29, 2017
2e8f105

typo


wesm Jul 29, 2017
b5a6d9a

Supress another clang warning


wesm Jul 30, 2017
dedcbb9

Fix typo


wesm Jul 30, 2017
9534ae9

Only pass PYARROW_CXXFLAGS when set


wesm Jul 30, 2017

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


3 changes: 3 additions & 0 deletions
ci/msvc-build.bat

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -114,6 +114,9 @@ popd
@rem see PARQUET-1018

pushd python

set PYARROW_CXXFLAGS=/WX
python setup.py build_ext --inplace --with-parquet --bundle-arrow-cpp bdist_wheel || exit /B
py.test pyarrow -v -s --parquet || exit /B

popd

1 change: 1 addition & 0 deletions
ci/travis_script_python.sh

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -23,6 +23,7 @@ source $TRAVIS_BUILD_DIR/ci/travis_env_common.sh
export ARROW_HOME=$ARROW_CPP_INSTALL
export PARQUET_HOME=$TRAVIS_BUILD_DIR/parquet-env
export LD_LIBRARY_PATH=$ARROW_HOME/lib:$PARQUET_HOME/lib:$LD_LIBRARY_PATH
export PYARROW_CXXFLAGS="-Werror"

build_parquet_cpp() {
 export PARQUET_ARROW_VERSION=$(git rev-parse HEAD)
Expand Down

11 changes: 11 additions & 0 deletions
python/CMakeLists.txt

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -57,6 +57,8 @@ if("${CMAKE_SOURCE_DIR}" STREQUAL "${CMAKE_CURRENT_SOURCE_DIR}")
 option(PYARROW_BUNDLE_ARROW_CPP
 "Bundle the Arrow C++ libraries"
 OFF)
 set(PYARROW_CXXFLAGS "" CACHE STRING
 "Compiler flags to append when compiling Arrow")
endif()

find_program(CCACHE_FOUND ccache)
Expand All@@ -75,13 +77,21 @@ include(CompilerInfo)

# Add common flags
set(CMAKE_CXX_FLAGS "${CXX_COMMON_FLAGS} ${CMAKE_CXX_FLAGS}")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${PYARROW_CXXFLAGS}")

if (NOT MSVC)
 # Enable perf and other tools to work properly
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-omit-frame-pointer")

 # Suppress Cython warnings
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-unused-variable")
else()
 # MSVC version of -Wno-return-type-c-linkage
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /wd4190")

 # Cython generates some bitshift expressions that MSVC does not like in
 # __Pyx_PyFloat_DivideObjC
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /wd4293")
endif()

if ("${COMPILER_FAMILY}" STREQUAL "clang")
Expand All@@ -95,6 +105,7 @@ if ("${COMPILER_FAMILY}" STREQUAL "clang")
 # Cython warnings in clang
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-parentheses-equality")
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-constant-logical-operand")
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-missing-declarations")

 # We have public Cython APIs which return C++ types, which are in an extern
 # "C" blog (no symbol mangling) and clang doesn't like this
Expand Down

1 change: 0 additions & 1 deletion
python/doc/source/development.rst

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -267,7 +267,6 @@ Now, we build and install Arrow C++ libraries
 -DCMAKE_INSTALL_PREFIX=%ARROW_HOME% ^
 -DCMAKE_BUILD_TYPE=Release ^
 -DARROW_BUILD_TESTS=off ^
 -DARROW_ZLIB_VENDORED=off ^
 -DARROW_PYTHON=on ..
 cmake --build . --target INSTALL --config Release
 cd ..\..
Expand Down

2 changes: 1 addition & 1 deletion
python/pyarrow/_parquet.pyx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -636,7 +636,7 @@ cdef class ParquetWriter:
 elif row_group_size == 0:
 raise ValueError('Row group size cannot be 0')

 cdef int c_row_group_size = row_group_size
 cdef int64_t c_row_group_size = row_group_size

 with nogil:
 check_status(self.writer.get()
Expand Down

2 changes: 1 addition & 1 deletion
python/pyarrow/includes/libarrow.pxd

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -369,7 +369,7 @@ cdef extern from "arrow/api.h" namespace "arrow" nogil:
 shared_ptr[CTable]* table)

 int num_columns()
int num_rows()
int64_t num_rows()

 c_bool Equals(const CTable& other)

Expand Down

2 changes: 1 addition & 1 deletion
python/pyarrow/io.pxi

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -923,7 +923,7 @@ cdef class _HdfsClient:
 cdef c_string c_path = tobytes(path)
 with nogil:
 check_status(self.client.get()
 .Delete(c_path, recursive))
 .Delete(c_path, recursive == 1))

 def open(self, path, mode='rb', buffer_size=None, replication=None,
 default_block_size=None):
Expand Down

1 change: 0 additions & 1 deletion
python/pyarrow/scalar.pxi

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -169,7 +169,6 @@ cdef class Time64Value(ArrayValue):
 CTime64Type* dtype = <CTime64Type*> ap.type().get()

 cdef int64_t val = ap.Value(self.index)
 print(val)
 if dtype.unit() == TimeUnit_MICRO:
 return (datetime.datetime(1970, 1, 1) +
 datetime.timedelta(microseconds=val)).time()
Expand Down

10 changes: 5 additions & 5 deletions
python/pyarrow/table.pxi

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -286,7 +286,7 @@ cdef int _schema_from_arrays(
 c_string c_name
 vector[shared_ptr[CField]] fields
 shared_ptr[CDataType] type_
int K = len(arrays)
Py_ssize_t K = len(arrays)

 fields.resize(K)

Expand DownExpand Up@@ -733,7 +733,7 @@ cdef class Table:
 vector[shared_ptr[CColumn]] columns
 shared_ptr[CSchema] schema
 shared_ptr[CTable] table
size_t K = len(arrays)
int i, K = <int> len(arrays)

 _schema_from_arrays(arrays, names, metadata, &schema)

Expand DownExpand Up@@ -841,7 +841,7 @@ cdef class Table:
 self._check_nullptr()
 return pyarrow_wrap_schema(self.table.schema())

 def column(self, int64_t i):
 def column(self, int i):
 """
 Select a column by its numeric index.

Expand All@@ -855,8 +855,8 @@ cdef class Table:
 """
 cdef:
 Column column = Column()
int64_t num_columns = self.num_columns
int64_t index
int num_columns = self.num_columns
int index

 self._check_nullptr()
 if not -num_columns <= i < num_columns:
Expand Down

8 changes: 4 additions & 4 deletions
python/pyarrow/types.pxi

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -281,12 +281,12 @@ cdef class Schema:
 def __len__(self):
 return self.schema.num_fields()

 def __getitem__(self, int64_t i):
 def __getitem__(self, int i):

 cdef:
 Field result = Field()
int64_t num_fields = self.schema.num_fields()
int64_t index
int num_fields = self.schema.num_fields()
int index

 if not -num_fields <= i < num_fields:
 raise IndexError(
Expand DownExpand Up@@ -456,7 +456,7 @@ def field(name, DataType type, bint nullable=True, dict metadata=None):
 convert_metadata(metadata, &c_meta)

 result.sp_field.reset(new CField(tobytes(name), type.sp_type,
 nullable, c_meta))
 nullable == 1, c_meta))
 result.field = result.sp_field.get()
 result.type = type
 return result
Expand Down

6 changes: 6 additions & 0 deletions
python/setup.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -92,6 +92,8 @@ def initialize_options(self):
 self.extra_cmake_args = os.environ.get('PYARROW_CMAKE_OPTIONS', '')
 self.build_type = os.environ.get('PYARROW_BUILD_TYPE', 'debug').lower()

 self.cmake_cxxflags = os.environ.get('PYARROW_CXXFLAGS', '')

 if sys.platform == 'win32':
 # Cannot do debug builds in Windows unless Python itself is a debug
 # build
Expand DownExpand Up@@ -146,6 +148,10 @@ def _run_cmake(self):
 if self.with_plasma:
 cmake_options.append('-DPYARROW_BUILD_PLASMA=on')

 if len(self.cmake_cxxflags) > 0:
 cmake_options.append('-DPYARROW_CXXFLAGS="{0}"'
 .format(self.cmake_cxxflags))

 if self.bundle_arrow_cpp:
 cmake_options.append('-DPYARROW_BUNDLE_ARROW_CPP=ON')
 # ARROW-1090: work around CMake rough edges
Expand Down

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode");
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();

Skip to content
ARROW-932: [Python] Fix MSVC compiler warnings, build Python with /WX and -Werror in CI#913

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.

 Sign up for GitHub

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

 Closed

wesm
wants to merge
6
commits into

from

 Closed


ARROW-932: [Python] Fix MSVC compiler warnings, build Python with /WX and -Werror in CI
#913
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


6 commits


Select commit
Hold shift + click to select a range
c32ee09

Remove print statement. Disable MSVC 4190 warning


wesm Jul 29, 2017
5740f00

Add PYARROW_CXXFLAGS option, fix MSVC compiler warnings


wesm Jul 29, 2017
2e8f105

typo


wesm Jul 29, 2017
b5a6d9a

Supress another clang warning


wesm Jul 30, 2017
dedcbb9

Fix typo


wesm Jul 30, 2017
9534ae9

Only pass PYARROW_CXXFLAGS when set


wesm Jul 30, 2017

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


3 changes: 3 additions & 0 deletions
ci/msvc-build.bat

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -114,6 +114,9 @@ popd
@rem see PARQUET-1018

pushd python

set PYARROW_CXXFLAGS=/WX
python setup.py build_ext --inplace --with-parquet --bundle-arrow-cpp bdist_wheel || exit /B
py.test pyarrow -v -s --parquet || exit /B

popd

1 change: 1 addition & 0 deletions
ci/travis_script_python.sh

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -23,6 +23,7 @@ source $TRAVIS_BUILD_DIR/ci/travis_env_common.sh
export ARROW_HOME=$ARROW_CPP_INSTALL
export PARQUET_HOME=$TRAVIS_BUILD_DIR/parquet-env
export LD_LIBRARY_PATH=$ARROW_HOME/lib:$PARQUET_HOME/lib:$LD_LIBRARY_PATH
export PYARROW_CXXFLAGS="-Werror"

build_parquet_cpp() {
 export PARQUET_ARROW_VERSION=$(git rev-parse HEAD)
Expand Down

11 changes: 11 additions & 0 deletions
python/CMakeLists.txt

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -57,6 +57,8 @@ if("${CMAKE_SOURCE_DIR}" STREQUAL "${CMAKE_CURRENT_SOURCE_DIR}")
 option(PYARROW_BUNDLE_ARROW_CPP
 "Bundle the Arrow C++ libraries"
 OFF)
 set(PYARROW_CXXFLAGS "" CACHE STRING
 "Compiler flags to append when compiling Arrow")
endif()

find_program(CCACHE_FOUND ccache)
Expand All@@ -75,13 +77,21 @@ include(CompilerInfo)

# Add common flags
set(CMAKE_CXX_FLAGS "${CXX_COMMON_FLAGS} ${CMAKE_CXX_FLAGS}")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${PYARROW_CXXFLAGS}")

if (NOT MSVC)
 # Enable perf and other tools to work properly
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-omit-frame-pointer")

 # Suppress Cython warnings
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-unused-variable")
else()
 # MSVC version of -Wno-return-type-c-linkage
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /wd4190")

 # Cython generates some bitshift expressions that MSVC does not like in
 # __Pyx_PyFloat_DivideObjC
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /wd4293")
endif()

if ("${COMPILER_FAMILY}" STREQUAL "clang")
Expand All@@ -95,6 +105,7 @@ if ("${COMPILER_FAMILY}" STREQUAL "clang")
 # Cython warnings in clang
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-parentheses-equality")
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-constant-logical-operand")
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-missing-declarations")

 # We have public Cython APIs which return C++ types, which are in an extern
 # "C" blog (no symbol mangling) and clang doesn't like this
Expand Down

1 change: 0 additions & 1 deletion
python/doc/source/development.rst

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -267,7 +267,6 @@ Now, we build and install Arrow C++ libraries
 -DCMAKE_INSTALL_PREFIX=%ARROW_HOME% ^
 -DCMAKE_BUILD_TYPE=Release ^
 -DARROW_BUILD_TESTS=off ^
 -DARROW_ZLIB_VENDORED=off ^
 -DARROW_PYTHON=on ..
 cmake --build . --target INSTALL --config Release
 cd ..\..
Expand Down

2 changes: 1 addition & 1 deletion
python/pyarrow/_parquet.pyx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -636,7 +636,7 @@ cdef class ParquetWriter:
 elif row_group_size == 0:
 raise ValueError('Row group size cannot be 0')

 cdef int c_row_group_size = row_group_size
 cdef int64_t c_row_group_size = row_group_size

 with nogil:
 check_status(self.writer.get()
Expand Down

2 changes: 1 addition & 1 deletion
python/pyarrow/includes/libarrow.pxd

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -369,7 +369,7 @@ cdef extern from "arrow/api.h" namespace "arrow" nogil:
 shared_ptr[CTable]* table)

 int num_columns()
int num_rows()
int64_t num_rows()

 c_bool Equals(const CTable& other)

Expand Down

2 changes: 1 addition & 1 deletion
python/pyarrow/io.pxi

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -923,7 +923,7 @@ cdef class _HdfsClient:
 cdef c_string c_path = tobytes(path)
 with nogil:
 check_status(self.client.get()
 .Delete(c_path, recursive))
 .Delete(c_path, recursive == 1))

 def open(self, path, mode='rb', buffer_size=None, replication=None,
 default_block_size=None):
Expand Down

1 change: 0 additions & 1 deletion
python/pyarrow/scalar.pxi

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -169,7 +169,6 @@ cdef class Time64Value(ArrayValue):
 CTime64Type* dtype = <CTime64Type*> ap.type().get()

 cdef int64_t val = ap.Value(self.index)
 print(val)
 if dtype.unit() == TimeUnit_MICRO:
 return (datetime.datetime(1970, 1, 1) +
 datetime.timedelta(microseconds=val)).time()
Expand Down

10 changes: 5 additions & 5 deletions
python/pyarrow/table.pxi

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -286,7 +286,7 @@ cdef int _schema_from_arrays(
 c_string c_name
 vector[shared_ptr[CField]] fields
 shared_ptr[CDataType] type_
int K = len(arrays)
Py_ssize_t K = len(arrays)

 fields.resize(K)

Expand DownExpand Up@@ -733,7 +733,7 @@ cdef class Table:
 vector[shared_ptr[CColumn]] columns
 shared_ptr[CSchema] schema
 shared_ptr[CTable] table
size_t K = len(arrays)
int i, K = <int> len(arrays)

 _schema_from_arrays(arrays, names, metadata, &schema)

Expand DownExpand Up@@ -841,7 +841,7 @@ cdef class Table:
 self._check_nullptr()
 return pyarrow_wrap_schema(self.table.schema())

 def column(self, int64_t i):
 def column(self, int i):
 """
 Select a column by its numeric index.

Expand All@@ -855,8 +855,8 @@ cdef class Table:
 """
 cdef:
 Column column = Column()
int64_t num_columns = self.num_columns
int64_t index
int num_columns = self.num_columns
int index

 self._check_nullptr()
 if not -num_columns <= i < num_columns:
Expand Down

8 changes: 4 additions & 4 deletions
python/pyarrow/types.pxi

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -281,12 +281,12 @@ cdef class Schema:
 def __len__(self):
 return self.schema.num_fields()

 def __getitem__(self, int64_t i):
 def __getitem__(self, int i):

 cdef:
 Field result = Field()
int64_t num_fields = self.schema.num_fields()
int64_t index
int num_fields = self.schema.num_fields()
int index

 if not -num_fields <= i < num_fields:
 raise IndexError(
Expand DownExpand Up@@ -456,7 +456,7 @@ def field(name, DataType type, bint nullable=True, dict metadata=None):
 convert_metadata(metadata, &c_meta)

 result.sp_field.reset(new CField(tobytes(name), type.sp_type,
 nullable, c_meta))
 nullable == 1, c_meta))
 result.field = result.sp_field.get()
 result.type = type
 return result
Expand Down

6 changes: 6 additions & 0 deletions
python/setup.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -92,6 +92,8 @@ def initialize_options(self):
 self.extra_cmake_args = os.environ.get('PYARROW_CMAKE_OPTIONS', '')
 self.build_type = os.environ.get('PYARROW_BUILD_TYPE', 'debug').lower()

 self.cmake_cxxflags = os.environ.get('PYARROW_CXXFLAGS', '')

 if sys.platform == 'win32':
 # Cannot do debug builds in Windows unless Python itself is a debug
 # build
Expand DownExpand Up@@ -146,6 +148,10 @@ def _run_cmake(self):
 if self.with_plasma:
 cmake_options.append('-DPYARROW_BUILD_PLASMA=on')

 if len(self.cmake_cxxflags) > 0:
 cmake_options.append('-DPYARROW_CXXFLAGS="{0}"'
 .format(self.cmake_cxxflags))

 if self.bundle_arrow_cpp:
 cmake_options.append('-DPYARROW_BUNDLE_ARROW_CPP=ON')
 # ARROW-1090: work around CMake rough edges
Expand Down