Fixed an error, Python3.7 compatibility, and opencv #35
Conversation
| @@ -97,6 +97,7 @@ $(OPENCV_DEPLOY): $(OPENCV_BUILD) virtualenv | |||
| cp $(OPENCV_BUILD) $(SITE_PACKAGES_DIR) | |||
|
|
|||
| opencv: $(OPENCV_DEPLOY) | |||
| $(PIP) install opencv-python | |||
There was a problem hiding this comment.
Should not be required since we actually compile it
There was a problem hiding this comment.
It's a wrapper not the library itself. I always installed it to use ZBarCam and thought it would be nice to install it where opencv is compiled, but I can remove it.
There was a problem hiding this comment.
Yes I would rather not use it, things seem to work smoothly without it as far as I know
| @@ -25,7 +25,7 @@ OPENCV_VERSION=4.0.1 | |||
| OPENCV_ARCHIVE=$(OPENCV_BASENAME).tar.gz | |||
| OPENCV_BASENAME=opencv-$(OPENCV_VERSION) | |||
| OPENCV_BUILD_LIB_DIR=$(OPENCV_BASENAME)/build/lib | |||
| OPENCV_BUILD=$(OPENCV_BUILD_LIB_DIR)/python$(PYTHON_MAJOR_VERSION)/cv2*.so | |||
| OPENCV_BUILD=$(OPENCV_BUILD_LIB_DIR)/cv2*.so | |||
There was a problem hiding this comment.
Good catch, but I think it's actually the line above that should be kept
There was a problem hiding this comment.
Do you mean that w should not use OPENCV_BUILD anymore ?
There was a problem hiding this comment.
No I mean you deleted the variable that we want to keep, the other should be removed and we should keep:
OPENCV_BUILD=$(OPENCV_BUILD_LIB_DIR)/python$(PYTHON_MAJOR_VERSION)/cv2*.so
Somehow at it on my compile cv2 library is dumped in: opencv-4.0.1/build/lib/python3/cv2.cpython-36m-x86_64-linux-gnu.so same thing on Travis.
For instance see this recent build log:
https://api.travis-ci.org/v3/job/501134779/log.txt
Scanning dependencies of target opencv_python3
make[3]: Leaving directory '/home/user/opencv-4.0.1/build'
make[3]: Entering directory '/home/user/opencv-4.0.1/build'
[100%] Building CXX object modules/python3/CMakeFiles/opencv_python3.dir/__/src2/cv2.cpp.o
[100%] Linking CXX executable ../../bin/opencv_interactive-calibration
make[3]: Leaving directory '/home/user/opencv-4.0.1/build'
[100%] Built target opencv_interactive-calibration
[100%] Linking CXX shared module ../../lib/python3/cv2.cpython-36m-x86_64-linux-gnu.so
make[3]: Leaving directory '/home/user/opencv-4.0.1/build'
[100%] Built target opencv_python3
make[2]: Leaving directory '/home/user/opencv-4.0.1/build'
make[1]: Leaving directory '/home/user/opencv-4.0.1/build'
cp opencv-4.0.1/build/lib/python3/cv2*.so venv/lib/python3.6/site-packages
There was a problem hiding this comment.
Ok, for some reason it doesn't output to the same path for me.
There was a problem hiding this comment.
Ouch OK that's annoying. For me as as well when it was on opencv2 it was in build/lib/cv2*.so.
Maybe we should try a hybrid approach with a comment explaining both path are somewhat possible. But I would keep that for later.
There was a problem hiding this comment.
Ok, found out that actuallly that there is build/lib/cv2.so and I thought this one was the lib we wanted to copy.
There was a problem hiding this comment.
Oh yes I remember I had the same issue when migrating to opencv4, so yes I also have that lib, but it's not the one being used at the end somehow. I didn't investigate why.
But good to know that you also have something in opencv-4.0.1/build/lib/python3/ that way we can keep the Makefile "simple"
|
Thanks I get the idea, two of the tests are failing with: See https://travis-ci.org/kivy-garden/garden.zbarcam/builds/501117072 for details. |
|
I've bumped Cython as you suggested and added python3.7 to both tox and Docker and tests are running smoothly. However now the CI may run for too long a be interrupted by Travis. I'll try pushing the branch nonetheless, but we should probably find a way around to it |
|
I'll remove the |
| @@ -15,7 +15,7 @@ SYSTEM_DEPENDENCIES= \ | |||
| wget | |||
| OS=$(shell lsb_release -si) | |||
| PYTHON_MAJOR_VERSION=3 | |||
| PYTHON_MINOR_VERSION=6 | |||
| PYTHON_MINOR_VERSION=$(shell python -c 'import sys;print(sys.version_info[1])') | |||
There was a problem hiding this comment.
I see the idea, and indeed we could probably improve that Makefile somehow, but this is currently hardcoded somehow on purpose.
Because for instance on Ubuntu 18.04 LTS default system Python is 2.7 (shame!). The purpose of this Makefile is to prepare the virtualenv. Do you see what I mean?
However I understand the need of wanting it to automatically prepare a Python3.7 environment for you without having to edit the Makefile. In this case you can use the built-in variable overriding mechanism of Makefile, e.g. via:
make PYTHON_MINOR_VERSION=7
That will do just what you need.
This PR bumps the Cython version installed to 0.28.2 and change the assignement of the
PYTHON_MINOR_VERSIONvar in the Makefile so that it is compatible with Python3.7, fixes a typo in the assignement ofOPENCV_BUILDin the Makefile, and installs theopencv-pythonpackage (needed to use opencv) aftercv2.sois built and copied in the virtualenv directory.