stanfordnlp / dspy

DSPy: The framework for programming—not prompting—foundation models
https://dspy-docs.vercel.app/
MIT License
13.9k stars 1.07k forks source link

Circular import and NameError in Cohere Module #785

Open DSamuelHodge opened 2 months ago

DSamuelHodge commented 2 months ago

Description: The cohere.py module in the dsp package is experiencing circular import and NameError issues.

The circular import occurs when the dsp.modules.lm module and the dsp.modules.cohere module try to import each other, leading to a partial initialization of one of the modules.

The NameError occurs when the cohere module is not imported successfully, and the code tries to access cohere.CohereAPIError in the @backoff.on_exception decorator.

To reproduce the issue:

  1. Install the dsp package.
  2. Import the Cohere class from dsp.modules.cohere.
  3. Create an instance of the Cohere class.

Expected behavior: The Cohere class should be imported and instantiated without any circular import or NameError issues.

Actual behavior: The code raises a NameError when trying to access cohere.CohereAPIError in the @backoff.on_exception decorator.

Traceback:

Traceback (most recent call last):
  File ".../dsp/modules/cohere.py", line 98, in Cohere
    (cohere.CohereAPIError,),
NameError: name 'cohere' is not defined

Possible solutions:

  1. Refactor the code to avoid circular imports between the dsp.modules.lm and dsp.modules.cohere modules.
  2. Handle the case when the cohere module is not imported successfully and use a generic Exception in the @backoff.on_exception decorator.

Additional information:

Please let me know if you need any further information or have any suggestions to resolve this issue.

nbqu commented 2 months ago

Maybe this issue is resolved in #46 and is just updated for v2.4.3. Does it work for you?

sebbyjp commented 2 months ago

Its broken still. I actually fixed it in #792

arnavsinghvi11 commented 2 months ago

Thanks @DSamuelHodge for raising the issue and @sebbyjp for the fix! @sebbyjp could you actually create a separate PR for this change just to separate it from the rest of the changes in #792 so we can push it out faster? I still think we need to import cohere as it is used for creating clients within the model functionality but would be good to check on this separate branch. Thanks!

habanoz commented 2 months ago

Please give this one priority. I cannot use dspy at all.