fix(represent): the represent is broken when user-given image. Fix it and support pathlib.Path for img_path
#917
Conversation
…port pathlib.Path
deepface/DeepFace.py
Outdated
| # -------------------------------- | ||
| if len(img.shape) == 4: | ||
| img = img[0] # e.g. (1, 224, 224, 3) to (224, 224, 3) | ||
| if len(img.shape) == 3: | ||
| img = cv2.resize(img, target_size) | ||
| img = np.expand_dims(img, axis=0) | ||
| # when represent is called from verify, this is already normalized | ||
| img = np.expand_dims(img, axis=0) # Why we remove a axis=0 previously and here expand one? |
There was a problem hiding this comment.
Here need author review. I don't know why should this, so I add a comment without change anything.
There was a problem hiding this comment.
face recognition models expect inputs like (1, 224, 224, 3). so, this is intended behaviour. please remove your comment.
There was a problem hiding this comment.
but Why we do img = img[0] in 726
There was a problem hiding this comment.
it is expanding. if your input is (224, 224, 3) then this makes (1, 224, 224, 3). your command drops a dimension on the other hand.
deepface/DeepFace.py
Outdated
| img = img_path.copy() | ||
| else: | ||
| raise ValueError(f"unexpected type for img_path - {type(img_path)}") | ||
| # Try load. If load error, will raise exception internal | ||
| img, _ = functions.load_image(img_path) |
There was a problem hiding this comment.
Because load_image in fact support many type, and have properly raise. so here I just use try-except logic -- just call it. if failed, it will raise, and it should has almost same effects.
deepface/DeepFace.py
Outdated
| if isinstance(img_path, str): | ||
| img = functions.load_image(img_path) | ||
| elif type(img_path).__module__ == np.__name__: | ||
| if type(img_path).__module__ == np.__name__: |
There was a problem hiding this comment.
I think we do not need to check type. load_image handles loading different type of inputs. So, we can get rid of the if and else block here.
There was a problem hiding this comment.
Previously I find the diffrence between here and load_image for copy. But after carefully look, no copy is ok. I'll do it.
deepface/commons/functions.py
Outdated
| @@ -94,6 +94,14 @@ def load_image(img): | |||
| if type(img).__module__ == np.__name__: | |||
| return img, None | |||
|
|
|||
| try: | |||
| # Test whether img is a Python3's Path. If hit, tranform to str to let following logic work. | |||
| from pathlib import Path | |||
There was a problem hiding this comment.
This is already imported in line 3: https://github.com/serengil/deepface/blob/master/deepface/commons/functions.py#L3C21-L3C25
This causes linting error in github actions:
************* Module deepface.commons.functions
deepface/commons/functions.py:99:8: W0621: Redefining name 'Path' from outer scope (line 3) (redefined-outer-name)
deepface/commons/functions.py:99:8: W0404: Reimport 'Path' (imported line 3) (reimported)
|
Added some comments. Also, linting step is failed: https://github.com/serengil/deepface/actions/runs/7205630424/job/19636358763?pr=917 |
Thanks for quickly response. I'll change it. |
|
updated. please check if you have time. |
|
Thank you for your contribution |
Background
I want to directly use the
representfunction on pre-detected face images.When I just pass the image path in pathlib.Path, it raise error on not-supported type. But I think it should support it because it's popular in Python3. OK, it's a tiny problem, and I just change the image path to str.
After given the image path in str, It still failed. Because
https://github.com/serengil/deepface/blob/master/deepface/DeepFace.py#L706
return a tuple of (img, image_name), but here we assign wrong.
What's more, the
regionis also wrong type: It should be a dict of x,y,w,h if fromextract_faces, but here is a list.Changes
I fixed them all, in detail:
representuser given imageAnd test is added on
representuser given image. Docs forreprenton return types is also updated.Please give me some advice if it's not ok.