-
Notifications
You must be signed in to change notification settings - Fork 5k
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
Document Migration to TransformMessages
#2247
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2247 +/- ##
===========================================
+ Coverage 38.14% 50.06% +11.92%
===========================================
Files 78 78
Lines 7865 7867 +2
Branches 1683 1821 +138
===========================================
+ Hits 3000 3939 +939
+ Misses 4615 3599 -1016
- Partials 250 329 +79
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR removes some significant chunks of code. I suggest use 3-step approach:
- Add documentation for new API, remove old code from documentation, and add an FAQ entry about the migration.
- Add depreciation notices to the old code and specify a future release version after which it will be removed.
- Remove the old code starting the target release version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for not including the comment above before. Excluding this point I think it is at a good state.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good! Thank you!
@ekzhu this PR is blocked by your change request. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks!
* wip * tweaks * undo fix * undo removal * adds to FAQ * modify docs * undo formatter * updates docs * update deprec notice in compressible agent * restore notebooks * giorgossideris comments * cleanup * resolve comments * improve english * improve english * cleanup --------- Co-authored-by: Eric Zhu <ekzhu@users.noreply.github.com>
Why are these changes needed?
This PR clarifies the migration from
TransformChatHistory
andCompressibleAgent
toTransformMessages
to users:TransformMessages
to handle long contexts.NOTE: #2256 handles some of the migration by linking the new notebooks in
website/docs/Examples.md
NOTE 2: I couldn't render the website locally (probably because I couldn't install quatro on a Fedora machine), so I'm not sure how the blog looks. I plan to try again using Docker later on.
Related issue number
Checks