dspy icon indicating copy to clipboard operation
dspy copied to clipboard

Published 20 hours ago • verified publisher icon stanfordnlp

feature(dspy): Add MyScale in Retrieve

Open usamajamil43 opened this issue 10 months ago • 6 comments

Add MyScaleRM to support retrieval for MyScaleDB:

  • MyScaleDB: https://github.com/myscale/myscaledb

usamajamil43 avatar Apr 09 '24 01:04 usamajamil43

Hi @usamajamil43, thanks for the contribution! left a few comments in the PR. Noticed some todos so feel free to mark the PR as ready to merge once the changes are made.

Couple of additional changes:

  1. can you run ruff check . --fix-only to fix the failing test?
  2. could you add documentation for MyScaleRM to the Retrieval Model Clients API? Feel free to follow some of the example RM documentation for reference.

arnavsinghvi11 avatar Apr 13 '24 01:04 arnavsinghvi11

Hey @arnavsinghvi11 , I have fixed the issues and added the documentation. Kindly, take a look

usamajamil43 avatar Apr 20 '24 07:04 usamajamil43

Thanks @usamajamil43 ! LGTM. Unfortunately, I cannot test the caches for this myself, but could you verify that with the existing behavior before pushing? Also, the PR has a few merge conflicts to resolve but should be ready to merge following that!

arnavsinghvi11 avatar May 15 '24 21:05 arnavsinghvi11

Hi @usamajamil43 , could you run ruff check . --fix-only and commit again? Ready to merge after that!

arnavsinghvi11 avatar May 21 '24 04:05 arnavsinghvi11

Hi, @arnavsinghvi11 Done!

usamajamil43 avatar May 22 '24 13:05 usamajamil43

Hi @usamajamil43 , can you rebase to the latest version on main? should fix the failing ruff test and ready to merge.

arnavsinghvi11 avatar May 25 '24 23:05 arnavsinghvi11

Hey @arnavsinghvi11,

I've tried to resolve the conflicts, but I'm still getting conflicts in files like lm.py and evaluate.py, which I haven't worked on. I'm not sure which changes to keep and which to discard in these files.

usamajamil43 avatar May 27 '24 09:05 usamajamil43

Hi @usamajamil43 , I believe if you rebase to the latest commit on the main branch and then run ruff, this should fix the existing error. I tried doing this by pulling your branch locally which resolved it, but I don't have access to push changes.

arnavsinghvi11 avatar May 31 '24 04:05 arnavsinghvi11

@arnavsinghvi11 Done!

usamajamil43 avatar Jun 02 '24 16:06 usamajamil43

@arnavsinghvi11 Can you please take a look? I was busy, so @ahsansaeed878 did the merge.

usamajamil43 avatar Jun 11 '24 20:06 usamajamil43

Thanks @usamajamil43 and @ahsansaeed878 for your patience with the PR! merged.

arnavsinghvi11 avatar Jun 13 '24 14:06 arnavsinghvi11