-
Notifications
You must be signed in to change notification settings - Fork 1.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
fix: update imports to allow global dspy.Google #419
Conversation
LGTM, running checks |
One minor question, and looks good to me. |
@@ -8,7 +8,8 @@ | |||
import google.generativeai as genai | |||
except ImportError: | |||
google_api_error = Exception | |||
print("Not loading Google because it is not installed.") | |||
# print("Not loading Google because it is not installed.") |
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.
Why commenting this out?
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 it stays, we print this every time anybody imports dspy, regardless of whether they are importing Google specifically. It's similar to how imports are managed on the Cohere side in: dsp/modules/cohere.py.
I imagine there may be a cleaner way throughout the project to manage optional dependencies.
Running some basic checks then merging |
I wish there was an automatic way to tell github "please merge but only if the checks pass" instead of just waiting for the checks. Maybe there is. |
Currently, the new Google LM class, does not have consistent imports with the remainder of the LMs.
ie. dspy.Cohere works, but dspy.Google does not.
This PR makes small import changes to ensure consistency.