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

community[patch]: add missing chunk parameter for _stream/_astream #17807

Merged
merged 1 commit into from Feb 21, 2024

Conversation

mackong
Copy link
Contributor

@mackong mackong commented Feb 20, 2024

  • Description: Add missing chunk parameter for _stream/_astream for some chat models, make all chat models in a consistent behaviour.
  • Issue: N/A
  • Dependencies: N/A

@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Feb 20, 2024
Copy link

vercel bot commented Feb 20, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
langchain ⬜️ Ignored (Inspect) Visit Preview Feb 21, 2024 2:34am

@dosubot dosubot bot added Ɑ: models Related to LLMs or chat model modules 🔌: cohere Primarily related to Cohere integrations 🤖:improvement Medium size change to existing code to handle new use-cases labels Feb 20, 2024
@eyurtsev eyurtsev changed the title community: add missing chunk parameter for _stream/_astream community[patch]: add missing chunk parameter for _stream/_astream Feb 20, 2024
@dosubot dosubot bot added the lgtm PR looks good. Use to confirm that a PR is ready for merging. label Feb 20, 2024
@eyurtsev
Copy link
Collaborator

eyurtsev commented Feb 20, 2024

@mackong thank you very much for the patch!

There's another issue with most of these models. They should be calling the callback prior to yielding the chunk rather than after yielding the chunk. If you have interest, we'd appreciate another PR for that as well :)

@eyurtsev eyurtsev self-assigned this Feb 20, 2024
@eyurtsev
Copy link
Collaborator

@mackong if your'e able to resolve the linting error, we can merge right away

@mackong
Copy link
Contributor Author

mackong commented Feb 21, 2024

@mackong if your'e able to resolve the linting error, we can merge right away

Fixed.

@mackong
Copy link
Contributor Author

mackong commented Feb 21, 2024

@mackong thank you very much for the patch!

There's another issue with most of these models. They should be calling the callback prior to yielding the chunk rather than after yielding the chunk. If you have interest, we'd appreciate another PR for that as well :)

OK, I'll fix it in another PR.

@eyurtsev
Copy link
Collaborator

merge when tests pass

@baskaryan baskaryan merged commit 3189109 into langchain-ai:master Feb 21, 2024
58 checks passed
@mackong mackong deleted the add-missing-chunk branch February 22, 2024 02:10
k8si pushed a commit to Mozilla-Ocho/langchain that referenced this pull request Feb 22, 2024
…angchain-ai#17807)

- Description: Add missing chunk parameter for _stream/_astream for some
chat models, make all chat models in a consistent behaviour.
- Issue: N/A
- Dependencies: N/A
al1p pushed a commit to al1p/langchain that referenced this pull request Feb 27, 2024
…angchain-ai#17807)

- Description: Add missing chunk parameter for _stream/_astream for some
chat models, make all chat models in a consistent behaviour.
- Issue: N/A
- Dependencies: N/A
haydeniw pushed a commit to haydeniw/langchain that referenced this pull request Feb 27, 2024
…angchain-ai#17807)

- Description: Add missing chunk parameter for _stream/_astream for some
chat models, make all chat models in a consistent behaviour.
- Issue: N/A
- Dependencies: N/A
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔌: cohere Primarily related to Cohere integrations 🤖:improvement Medium size change to existing code to handle new use-cases lgtm PR looks good. Use to confirm that a PR is ready for merging. Ɑ: models Related to LLMs or chat model modules size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants