Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Added missing "mediapipe" dependency for mediapipe_face_common.py #39

Merged
merged 3 commits into from
May 22, 2023

Conversation

@jinwonkim93
Copy link
Contributor

jinwonkim93 commented May 19, 2023

Check #29. Did not include on setup because it might cause protobuf version conflict

@Ambrosiussen
Copy link
Contributor Author

But then why is it part of ControlNet AUX if it doesnt work??

@jinwonkim93
Copy link
Contributor

You can install it seperately by pip install mediapipe

@Ambrosiussen
Copy link
Contributor Author

Yes, I did that locally. But generally bad practice to ship code that needs a dependency not provided in a requirements.txt or setup. Maybe try to catch the exception when instantiating a mediapipe_face with the instructions on installing the missing dependency? (Or even better, add a requirements.txt)

@patrickvonplaten
Copy link
Contributor

Ideally we would throw a warning if mediapipe control is imported but mediapipe is not installed. Would be happy if we can not explode the dependency tree of this package

@Ambrosiussen
Copy link
Contributor Author

@patrickvonplaten Thoughts on adjusted code?

@patrickvonplaten
Copy link
Contributor

Perfect! Thanks!

@patrickvonplaten patrickvonplaten merged commit 9408286 into huggingface:master May 22, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants