-
Notifications
You must be signed in to change notification settings - Fork 11
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
models_update (update 1 of 2) #34
base: main
Are you sure you want to change the base?
Conversation
- model weights added - face_recog feature added and integrated - structured functions and files in required format - requirements and gitignore updated
Reviewer's Guide by SourceryThis PR introduces face recognition capabilities and gesture classification functionality while restructuring the project. The implementation uses DeepFace for face recognition and MediaPipe for hand gesture detection, with model weights being added to support these features. The changes include proper file organization and necessary dependency updates. ER diagram for updated requirementserDiagram
REQUIREMENTS {
string numpy
string pandas
string matplotlib
string scikit-learn
string mediapipe
string opencv-python
string tf-keras
string deepface
string flask
}
note for REQUIREMENTS "Updated to include new dependencies for face recognition and gesture classification"
Class diagram for gesture classification and face recognitionclassDiagram
class gestureClassify {
+detectGesture(img)
+compare(detectOutput, gestureClass)
}
class faceRecog {
+recog(img_1_path, img_2_path)
}
note for gestureClassify "Uses MediaPipe for hand gesture detection"
note for faceRecog "Uses DeepFace for face recognition"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey @BlakkTyger - I've reviewed your changes - here's some feedback:
Overall Comments:
- The model.zip extraction in gestureClassify.py happens on every function call, which is inefficient. Consider loading the model once during initialization.
- There's a bug in the compare() function where a bare 'True' statement doesn't return anything. This needs to be fixed to return True when gestures match.
Here's what I looked at during the review
- 🟡 General issues: 3 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
''' | ||
|
||
|
||
with ZipFile("./model.zip", 'r') as zObject: |
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.
suggestion (performance): Consider caching the model instead of extracting it on every function call
Repeatedly extracting the zip file for each detection will cause significant performance overhead. Consider loading the model once at module level or implementing a caching mechanism.
MODEL_ZIP = ZipFile("./model.zip", 'r')
def __init__(self):
self.model_object = MODEL_ZIP
if detectOutput == 1: | ||
return False, 'Gesture Not Inferred' | ||
else: | ||
if detectOutput == gestureClass: |
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.
issue (bug_risk): Missing return statement in the True case
The function will fall through to return False even when the gestures match. Add 'return True' here.
|
||
Parameters: | ||
img_1_path: path of image to be checked | ||
imt_2_path: path of original image |
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.
nitpick (typo): Fix typo in parameter name in docstring
'imt_2_path' should be 'img_2_path'
from zipfile import ZipFile | ||
|
||
|
||
def detectGesture(img): |
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.
issue (complexity): Consider refactoring model loading and return patterns to reduce redundant operations
Two suggestions to reduce complexity while maintaining functionality:
- Move model loading to module level to avoid repeated ZIP operations:
# At module level
_model = None
def _load_model():
global _model
if _model is None:
with ZipFile("./model.zip", 'r') as zObject:
zObject.extract("model.p", path="./")
_model = pickle.load(open('./model.p', 'rb'))['model']
return _model
# In detectGesture(), replace model loading with:
model = _load_model()
- Simplify compare() with consistent return types:
def compare(detectOutput, gestureClass):
if detectOutput == 0:
return False, 'Hand Not Detected'
if detectOutput == 1:
return False, 'Gesture Not Inferred'
return (detectOutput == gestureClass,
'Correct Gesture' if detectOutput == gestureClass else 'Incorrect Gesture')
These changes improve performance by loading the model once and make the code more maintainable with consistent return patterns.
🧙 Sourcery is reviewing your pull request! Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
We have skipped reviewing this pull request. It looks like we've already reviewed the commit 0ccef39 in this pull request.
Edits Made
To-Do in update 2 of 2
Summary by Sourcery
Add face recognition feature and integrate it into the system. Restructure codebase to meet required format and update dependencies in requirements.txt.
New Features:
Enhancements:
Build: