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

Unable to create new Provider if model_id_key="model_id" #258

Closed
michaelchia opened this issue Jul 10, 2023 · 3 comments · Fixed by #263
Closed

Unable to create new Provider if model_id_key="model_id" #258

michaelchia opened this issue Jul 10, 2023 · 3 comments · Fixed by #263
Labels
enhancement New feature or request

Comments

@michaelchia
Copy link
Collaborator

Problem

  • I am unable to create my own provider if the model_id_key for my langchain LLM object is "model_id".
    • the "model_id" kwarg would be repeated twice in super().__init__ in L122
      def __init__(self, *args, **kwargs):
      try:
      assert kwargs["model_id"]
      except:
      raise AssertionError(
      "model_id was not specified. Please specify it as a keyword argument."
      )
      model_kwargs = {}
      model_kwargs[self.__class__.model_id_key] = kwargs["model_id"]
      super().__init__(*args, **kwargs, **model_kwargs)
  • Not sure if this problem exists for BaseEmbeddingProvider as it seems to be implemented differently.

Proposed Solution

change L120 to:

if self.__class__.model_id_key != "model_id":
    model_kwargs[self.__class__.model_id_key] = kwargs["model_id"]
@michaelchia michaelchia added the enhancement New feature or request label Jul 10, 2023
@dlqqq
Copy link
Member

dlqqq commented Jul 11, 2023

@michaelchia I wouldn't expect that to raise an exception, since unpacking two dictionaries with shared keys does not; it only results in the second dictionary clobbering the first:

>>> a = { "key": 1 }
>>> b = { "key": 2 }
>>> c = { **a, **b }
>>> c
{'key': 2}

If this is raising an exception, could you post the exception info here? If not, I'm not clear on what the issue is.

@michaelchia
Copy link
Collaborator Author

Repeated keys are allowed in curly brackets but not for functions. e.g. {"key": 1, "key": 2} won't throw errors but something like dict(key=1, key=2) (or dict(**a, **b) as per your example) will throw an exception (TypeError: dict() got multiple values for keyword argument 'key'). if it were super().__init__(*args, **{**kwargs, **model_kwargs}) it would've been fine.

@3coins
Copy link
Collaborator

3coins commented Jul 12, 2023

@michaelchia
I have encountered this issue as well, and attempted to resolve this in PR #263.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants