Skip to content
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

Feat task 0203 enhancements #1059

Merged
merged 10 commits into from Mar 2, 2024
Merged

Feat task 0203 enhancements #1059

merged 10 commits into from Mar 2, 2024

Conversation

serengil
Copy link
Owner

@serengil serengil commented Mar 2, 2024

Tickets

What has been done

With this PR,

1- we were not giving meaningful message if face could not been detected for a given numpy array or base64 encoded image in verify function. this is sorted.
2- in verify function, if one of image pair is having many faces, we were calculating the embedding again and again unncessary.
3- if an image name starts with http, then we were attempting to load it from web
4- in find, we were enforcing images with jpg, jpeg and png but this is not checked for base64 encoded images. to make it compatible, we are now checking type of base64.
5- fastmtcnn detector was throwing error if face could not been detected.
6- stock image of detectors updated.

How to test

make lint && make test

@@ -34,7 +34,7 @@ def load_image(img: Union[str, np.ndarray]) -> Tuple[np.ndarray, str]:
return load_base64(img), "base64 encoded string"

# The image is a url
if img.startswith("http"):
if img.startswith("http://") or img.startswith("https://"):
Copy link
Contributor

Choose a reason for hiding this comment

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

AFAIK python's string comparison is case sensisitve. Hance this might miss upper case HTTP:// and all combinations in between. Maybe transform to lower case before ? (as you seem to don't like regexes)

Copy link
Owner Author

Choose a reason for hiding this comment

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

morning, you are absolutely right. cheers!

creating an issue to follow this.

@serengil serengil mentioned this pull request Mar 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants