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

Support abstract LanguageModel-class to integrate with other APIs #997

Merged
merged 1 commit into from
Aug 20, 2024

Conversation

NickyHavoc
Copy link
Collaborator

@NickyHavoc NickyHavoc commented Aug 19, 2024

Description

No description.

Before Merging

  • Review the code changes
    • Unused print / comments / TODOs
    • Missing docstrings for functions that should have them
    • Consistent variable names
    • ...
  • Update changelog.md if necessary
  • Commit messages should contain a semantic label and the ticket number
    • Consider squashing if this is not the case

@NickyHavoc NickyHavoc force-pushed the abc-for-llm branch 2 times, most recently from 3aaead6 to 97bbda6 Compare August 19, 2024 14:14
@NickyHavoc NickyHavoc changed the title Support abstract LargeLanguageModel-class to integrate with other APIs Support abstract LanguageModel-class to integrate with other APIs Aug 19, 2024
@NickyHavoc NickyHavoc force-pushed the abc-for-llm branch 2 times, most recently from ce08704 to a7fca73 Compare August 19, 2024 15:30
Copy link
Contributor

@SebastianNiehusAA SebastianNiehusAA left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking great overall. Please just move the changelog entry to the appropriate section and add the docstrings where marked and we are good to go :)

CHANGELOG.md Outdated Show resolved Hide resolved
src/intelligence_layer/core/model.py Show resolved Hide resolved
src/intelligence_layer/core/model.py Show resolved Hide resolved
@NickyHavoc NickyHavoc force-pushed the abc-for-llm branch 2 times, most recently from a93e0fa to 9e51f8c Compare August 20, 2024 09:16
Copy link
Contributor

@SebastianNiehusAA SebastianNiehusAA left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-lint and then we are fine

adjust changelog

linting

hopefully improve flaky QA test

hopefully fix test

address PR comments

revert change in SingleChunkQa

linting
@NickyHavoc NickyHavoc merged commit b352e35 into main Aug 20, 2024
4 checks passed
@NickyHavoc NickyHavoc deleted the abc-for-llm branch August 20, 2024 09:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants