dspy icon indicating copy to clipboard operation
dspy copied to clipboard

Adds native support for Claude models

Open skucherlapati opened this issue 11 months ago • 2 comments

Adds native support for claude wrapper

skucherlapati avatar Mar 05 '24 19:03 skucherlapati

Thank you.

It would be great if this files is updated, setup.py for package requirement Also it would good to add the documentation in this folder: docs/api/language_model_clients

insop avatar Mar 07 '24 03:03 insop

Thanks a lot! No don't update setup.py, this does not need to be a global requirement.

Instead the import should happen only if needed, e.g. inside __init___

okhat avatar Mar 07 '24 03:03 okhat

@insop could you please help me with the ruff check here? I tried with:

ruff check . --fix-only

CShorten avatar Mar 09 '24 18:03 CShorten

@insop could you please help me with the ruff check here? I tried with:

ruff check . --fix-only

I am new to ruff as I am still use black.

But I gave it a try by merging this PR branch to current main a51aad7dcbe257e01d677025cad96acba7c13e90.

Here is what I found:

  • main branch a51aad7dcbe257e01d677025cad96acba7c13e90: no ruff issue
  • my test branch with this PR merged to main: has many ruff issues that are not part of the PR (I double checked my test merging branch)

It is appeared to be that there is some config file change or some type of cached information are playing here?

Sorry, I tried, not no answer to this.

insop avatar Mar 09 '24 19:03 insop

Ok, so good to merge this PR without passing the ruff test?

Lgtm otherwise, maybe before every release we run the ruff test on master until this is fully understood within the team going forward?

CShorten avatar Mar 09 '24 19:03 CShorten

Looking into this now. Do not merge yet, the test failing is not CI fault

isaacbmiller avatar Mar 09 '24 19:03 isaacbmiller

@CShorten How are you pushing to this pull request?

isaacbmiller avatar Mar 09 '24 19:03 isaacbmiller

Okay I fixed the issues:

  1. You need to make the imports optional, as anthropic is not a required dependency. That is why the tests were failing
  2. Somehow, there was a deletion of a Ruff rule in pyproject.toml, which is where there were so many fixes. Sorry for the mess on your branch!

Good to merge whenever @okhat @CShorten

isaacbmiller avatar Mar 09 '24 20:03 isaacbmiller

Thank you so much @insop @isaacbmiller @skucherlapati @okhat! Really appreciate your contributions to this! Claude API in DSPy!! Super cool!

CShorten avatar Mar 09 '24 20:03 CShorten

Thank you so much @insop !! Really appreciate your contribution to this! Claude API in DSPy!! Super cool!

Thank you @CShorten, happy to see Claude API in DSPy as well.

insop avatar Mar 09 '24 21:03 insop

Thanks for the help on getting this merged. Makes sense - I should have made it optional!

skucherlapati avatar Mar 15 '24 17:03 skucherlapati