Migrating the count token guide to the unified SDK #467
Conversation
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
github-actions
bot
added
status:awaiting review
|
View / edit / reply to this conversation on ReviewNB markmcd commented on 2025-02-20T09:47:19Z If we don't want to pin versions when installing packages, it might be helpful to at least print the package version that got installed. Just as a way to help users track version skew without adding maintenance burden. Giom-V commented on 2025-02-21T08:34:57Z Is that ok with you if I just set Giom-V commented on 2025-02-21T08:41:22Z I've added |
|
View / edit / reply to this conversation on ReviewNB markmcd commented on 2025-02-20T09:47:20Z These links are for the old SDK. Giom-V commented on 2025-02-21T08:41:33Z Good catch, it should be fixed now. |
|
View / edit / reply to this conversation on ReviewNB markmcd commented on 2025-02-20T09:47:21Z currently 258 tokens
"currently" indicates that we might change it, which I don't think we want to say.
My preference for these cases is "show, don't tell" - i.e. don't write down the token count in the text, but demonstrate it using code. This way if it ever changes, the text is still correct.
Giom-V commented on 2025-02-21T09:02:54Z I have mixed feeling about that as it a question I got multiple times from devs, but I still updated the text to point to the documentation. |
|
View / edit / reply to this conversation on ReviewNB markmcd commented on 2025-02-20T09:47:22Z Line #3. contents=["Tell me about this instrument",organ]
Giom-V commented on 2025-02-21T08:42:59Z Fixed. Thanks! |
|
View / edit / reply to this conversation on ReviewNB markmcd commented on 2025-02-20T09:47:23Z As you can see
The number output above doesn't match the number here. There is some skew that the reader has to figure out for themselves. If you want to show the fixed size of an image, call Giom-V commented on 2025-02-21T09:03:09Z Updated the example. |
|
View / edit / reply to this conversation on ReviewNB markmcd commented on 2025-02-20T09:47:24Z I realise it wasn't here before, so optional, but it'd be nice to show how long the MP3 file is so the user can easily do the tok/sec calculation.
eg !ffprobe -v error -show_entries format=duration sample.mp3 Giom-V commented on 2025-02-21T09:09:13Z Good idea. I've added it. |
|
View / edit / reply to this conversation on ReviewNB markmcd commented on 2025-02-20T09:47:24Z Did you mean to delete the system instruction & tools section? It is useful to understand how they contribute to the overall context. Giom-V commented on 2025-02-21T08:39:26Z I agree but they don't work at the moment. I've reported the issue to the SDK team and create fast-follow tasks to add back that content when it will be supported. I'll add a line about it in the commit and the PR description. Giom-V commented on 2025-02-21T09:09:12Z I also added a note in the notebook itself. |
|
Is that ok with you if I just set View entire conversation on ReviewNB |
|
I agree but they don't work at the moment. I've reported the issue to the SDK team and create fast-follow tasks to add back that content when it will be supported. I'll add a line about it in the commit and the PR description. View entire conversation on ReviewNB |
|
I've added View entire conversation on ReviewNB |
|
Good catch, it should be fixed now. View entire conversation on ReviewNB |
|
Fixed. Thanks! View entire conversation on ReviewNB |
|
I have mixed feeling about that as it a question I got multiple times from devs, but I still updated the text to point to the documentation. View entire conversation on ReviewNB |
|
Updated the example. View entire conversation on ReviewNB |
|
I also added a note in the notebook itself. View entire conversation on ReviewNB |
|
Good idea. I've added it. View entire conversation on ReviewNB |
* Migrating the count token guide to the unified SDK * Chat, tools and caching are not supported at the moment, the examples have been removed and will be added back when they will be supported.
No description provided.