-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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.
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
Add basic image io and remove python C-API refs from numpy_returns.cpp #1258
Conversation
@visionworkz I started a local branch (with ifdef stuff) this weekend but haven't finished it yet. I'll wait for this batch of work to get merged in first then. |
@visionworkz, all the file paths when running |
@@ -24,17 +34,26 @@ def get_test_face_chips(): | |||
def get_test_face_chip(): | |||
rgb_img, shape = get_test_image_and_shape() | |||
return dlib.get_face_chip(rgb_img, shape) | |||
|
|||
|
|||
# The tests below will be skipped for Python 2.7 as there is a pickle issue |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should work in python 2.7. Can't you pickle using an earlier pickling version, like 2? It's certainly not the case that pybind11 pickling doesn't work in python 2.7.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@davisking I tried a few things:
pybind/pybind11#271
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you use cPickle and a call like .dumps(variable,2) it works. See http://pybind11.readthedocs.io/en/stable/advanced/classes.html#pickling-support
Looks awesome, thanks for the nice PR :) |
davisking#1258) * Fixed reference count issue * Fixed refcount issue in Python dlib.jitter_image and dlib.get_face_chips * Consolidation of davisking#1249 * Fixed build issue * Fixed: Paths in a pytest file should be relative to dlib root * Skip numpy return tests for Python 2.7 or if Numpy is not installed * Enabled numpy returns tests on Python 2.7 using cPickle.dumps
This is a consolidation of the discussion in #1249
For your review @davisking and @supervacuus
@davisking I tried to fix the build but it cannot load the image in the test folder. I verified that the image exists. Maybe an issue related to DLIB_JPEG_SUPPORT flag?
There is further impact due to #1252
@aldanor if this PR is merged before your PR, you might wish to
ifdef
some portions of the code in this PR as well.