Skip to content

Attempt porting to python 3. #25

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

Open
bluescarni wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Attempt porting to python 3. #25

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
56 changes: 29 additions & 27 deletions python/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,39 +6,41 @@ cmake_minimum_required (VERSION 2.8)
#
######################################################################

FIND_PACKAGE(PythonInterp 2 REQUIRED)

IF(PYTHONINTERP_FOUND)
# check that Python version 2.x is used
execute_process(COMMAND ${PYTHON_EXECUTABLE} -c
"import sys; print(sys.version[0])"
OUTPUT_VARIABLE PYTHON_MAJOR_VERSION OUTPUT_STRIP_TRAILING_WHITESPACE)
IF(NOT (${PYTHON_MAJOR_VERSION} EQUAL 2))
MESSAGE(FATAL_ERROR "Python bindings require Python 2.x.")
ENDIF()
ENDIF()
FIND_PACKAGE(PythonInterp REQUIRED)

#IF(PYTHONINTERP_FOUND)
# # check that Python version 2.x is used
# execute_process(COMMAND ${PYTHON_EXECUTABLE} -c
# "import sys; print(sys.version[0])"
# OUTPUT_VARIABLE PYTHON_MAJOR_VERSION OUTPUT_STRIP_TRAILING_WHITESPACE)
# IF(NOT (${PYTHON_MAJOR_VERSION} EQUAL 2))
# MESSAGE(FATAL_ERROR "Python bindings require Python 2.x.")
# ENDIF()
#ENDIF()

@jakirkham jakirkham May 23, 2017

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this can just be dropped, right?


######################################################################
#
# find Python library
#
######################################################################

execute_process(COMMAND ${PYTHON_EXECUTABLE} -c
"import sys; print sys.exec_prefix"
OUTPUT_VARIABLE PYTHON_PREFIX OUTPUT_STRIP_TRAILING_WHITESPACE)
find_package(PythonLibs REQUIRED)

IF(APPLE AND ${PYTHON_PREFIX} MATCHES ".*framework.*")
SET(PYTHON_LIBRARIES "${PYTHON_PREFIX}/Python"
CACHE FILEPATH "Python libraries"
FORCE)
ELSE()
execute_process(COMMAND ${PYTHON_EXECUTABLE} -c
"import sys; skip = 2 if sys.platform.startswith('win') else 1; print 'python' + sys.version[0:3:skip]"
OUTPUT_VARIABLE PYTHON_LIBRARY_NAME OUTPUT_STRIP_TRAILING_WHITESPACE)
FIND_LIBRARY(PYTHON_LIBRARIES ${PYTHON_LIBRARY_NAME} HINTS "${PYTHON_PREFIX}"
PATH_SUFFIXES lib lib64 libs DOC "Python libraries")
ENDIF()
#execute_process(COMMAND ${PYTHON_EXECUTABLE} -c
# "import sys; print(sys.exec_prefix)"
# OUTPUT_VARIABLE PYTHON_PREFIX OUTPUT_STRIP_TRAILING_WHITESPACE)

#IF(APPLE AND ${PYTHON_PREFIX} MATCHES ".*framework.*")
# SET(PYTHON_LIBRARIES "${PYTHON_PREFIX}/Python"
# CACHE FILEPATH "Python libraries"
# FORCE)
#ELSE()
# execute_process(COMMAND ${PYTHON_EXECUTABLE} -c
# "import sys; skip = 2 if sys.platform.startswith('win') else 1; print('python' + sys.version[0:3:skip])"
# OUTPUT_VARIABLE PYTHON_LIBRARY_NAME OUTPUT_STRIP_TRAILING_WHITESPACE)
# FIND_LIBRARY(PYTHON_LIBRARIES ${PYTHON_LIBRARY_NAME} HINTS "${PYTHON_PREFIX}"
# PATH_SUFFIXES lib lib64 libs DOC "Python libraries")
#ENDIF()

@jakirkham jakirkham May 23, 2017

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same story regarding dropping.

@jakirkham jakirkham May 23, 2017 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should add that while I haven't built this package before, I have had some rough experiences with CMake finding the right Python. If this works, great, but it might be worth checking with others who added this to verify it still works.


######################################################################
#
Expand All @@ -47,7 +49,7 @@ ENDIF()
######################################################################

execute_process(COMMAND ${PYTHON_EXECUTABLE} -c
"from distutils.sysconfig import *; print get_python_inc()"
"from distutils.sysconfig import *; print(get_python_inc())"
OUTPUT_VARIABLE PYTHON_INCLUDE OUTPUT_STRIP_TRAILING_WHITESPACE)
SET(PYTHON_INCLUDE_PATH ${PYTHON_INCLUDE}
CACHE PATH "Path to Python include files"
Expand All @@ -61,7 +63,7 @@ SET(PYTHON_INCLUDE_PATH ${PYTHON_INCLUDE}
######################################################################
IF(NOT DEFINED IIBOOST_PYTHON_INSTALL_DIR OR IIBOOST_PYTHON_INSTALL_DIR MATCHES "^$")
execute_process(COMMAND ${PYTHON_EXECUTABLE} -c
"from distutils.sysconfig import *; print get_python_lib(1)"
"from distutils.sysconfig import *; print(get_python_lib(1))"
OUTPUT_VARIABLE PYTHON_SITE_PACKAGES OUTPUT_STRIP_TRAILING_WHITESPACE)
FILE(TO_CMAKE_PATH ${PYTHON_SITE_PACKAGES} IIBOOST_PYTHON_INSTALL_DIR)
ENDIF()
Expand Down
5 changes: 4 additions & 1 deletion python/iiboost/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1,4 @@
from booster import Booster, EigenVectorsOfHessianImage, computeEigenVectorsOfHessianImage, computeIntegralImage, ROICoordinates
# for python 2.0 compatibility
from __future__ import absolute_import as _ai

@jakirkham jakirkham May 23, 2017

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the aliasing?


from .booster import Booster, EigenVectorsOfHessianImage, computeEigenVectorsOfHessianImage, computeIntegralImage, ROICoordinates
5 changes: 3 additions & 2 deletions python/iiboost/booster.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@
import numpy as np
import ctypes

from exceptions import RuntimeError
if sys.version_info[0] < 3:
from exceptions import RuntimeError

@jakirkham jakirkham May 23, 2017

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this even need to be imported at all? FWICT this is already available on Python 2.7 and 2.6 without importing.

@jakirkham jakirkham May 23, 2017

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also might be worth adding from __future__ import print_function here as well.


# libiiboost_python.so must reside in the same directory as this module.
if sys.platform.startswith('win'):
Expand Down Expand Up @@ -277,7 +278,7 @@ def trainWithChannels( self, imgStackList, eigVecOfHessianImgList,

numStacks = len(chStackListList)
numChannels = len(chStackListList[0])
print "Number of stacks: ",numStacks,". Each with ",numChannels," channels."
print("Number of stacks: ",numStacks,". Each with ",numChannels," channels.")

if debugOutput:
dbgOut = ctypes.c_int(1)
Expand Down
2 changes: 1 addition & 1 deletion python/src/iiboost_python.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ extern "C"
std::string str;
((BoosterModel *)modelPtr)->serializeToString( &str );

return PyString_FromString( str.c_str() );
return PyBytes_FromString( str.c_str() );
}

// create model from serialized string
Expand Down
10 changes: 5 additions & 5 deletions python/tests/python_test_raw.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,19 +20,19 @@ def propToCArray( L, prop, cArrayElType ):


# load data
print "--- Loading data ---"
print("--- Loading data ---")

@jakirkham jakirkham May 23, 2017

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be worth adding from __future__ import print_function as the first line above to make sure this behavior is maintained even on Python 2.

gts = [joblib.load("../../testData/gt.jlb")]
imgs = [joblib.load("../../testData/img.jlb")]


print "--- Loading lib ---"
print("--- Loading lib ---")
boostLib = ctypes.CDLL("../../build/python/libiiboost_python.so")

# this returns a python string
boostLib.serializeModel.restype = ctypes.py_object


print "--- Calling train() ---"
print("--- Calling train() ---")

# we need to pass an array to the C call
widthList = propToCArray( imgs, "shape[2]", ctypes.c_int)
Expand All @@ -53,13 +53,13 @@ def propToCArray( L, prop, cArrayElType ):
ctypes.c_int(numStumps),
ctypes.c_int(debugOutput) ) )

print "--- Serializing model ---"
print("--- Serializing model ---")
serStr = ctypes.py_object( boostLib.serializeModel( model ) )

# pre-alloc prediction
pred = np.empty_like( imgs[0], dtype=np.dtype("float32") )

print "--- Predicting ---"
print("--- Predicting ---")
boostLib.predict( model,
ctypes.c_void_p(imgs[0].ctypes.data),
widthList[0], heightList[0], depthList[0],
Expand Down