Backend Refactor [WIP] #424
Conversation
New Signature class based on pydantic + unit tests
|
@okhat @arnavsinghvi11 are we good to merge this one? |
|
Hey everyone, just catching up on this. Let's prioritize merging very soon (let's say within 2 days). Biggest problem right now is that I gather (but not sure) that 90% of the files changed are just formatting. Is this accurate? It makes it harder to review this PR. Do we need these formatting changes? Isn't ruff supposed to take that out of the story? Basically I'd love to review a version of this PR that's mostly or entirely actual updates so nothing slips. |
|
That is correct, we did our best but its messy. Without rewriting this PR from scratch, I dont think we can make it cleaner. The repo went through so many major changes during this period and the merges got a bit clobbered. Ive outlined the three areas to review above. It should just be the modeling/predict and primitives/prediction modules. Ive extensively tested this and it should be fine. I would love to see this merged within 48 hours. Its not perfect, and there are additional issues we can fix on follow up, but existing functionality remains unchanged. With CI and consistent formatting, I think all future changes will be much better scoped and focused. Please let me know whats needed to get this over the line. |
| self.kwargs.pop("frequency_penalty", None) | ||
| self.kwargs.pop("presence_penalty", None) |
There was a problem hiding this comment.
is it worth having a seperate "run ruff on everything" PR? The pre-commit hooks only look at changed files so source which predates ruff is still unformatted.
|
Hi @okhat any update on when this can be merged? We use Instructor for a few things, and I'd like to create an InstructorBackend for ourselves instead of creating a new |
No description provided.