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

Implement AsyncTextIteratorStreamer for asynchronous streaming #34931

Merged
merged 20 commits into from
Dec 20, 2024

Conversation

CISC
Copy link
Contributor

@CISC CISC commented Nov 25, 2024

Asynchronous text generation streaming

Added a new AsyncTextIteratorStreamer class that allows you to have fully asynchronous text generation streaming in your application. Works identically to TextIteratorStreamer except that you iterate the stream as follows:

generated_text = ""
async for new_text in streamer:
    generated_text += new_text

Requires Python 3.11+ due to use of asyncio.timeout to handle timeout. Falls back to asyncio.wait_for if asyncio.timeout is not available.

Before submitting

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • Did you read the contributor guideline,
    Pull Request section?
  • Was this discussed/approved via a Github issue or the forum? Please add a link
    to it if that's the case.
  • Did you make sure to update the documentation with your changes? Here are the
    documentation guidelines, and
    here are tips on formatting docstrings.
  • Did you write any new necessary tests?

Any hints on how best to implement an async test would be appreciated. :) pytest-asyncio seems to be the way to go...

Who can review?

@gante @ArthurZucker

@CISC
Copy link
Contributor Author

CISC commented Nov 25, 2024

Hmmm, looks like I'm missing something important regarding documentation, not sure what though?

Exception: The following objects are in the public init so should be documented:

  • AsyncTextIteratorStreamer

Was missing autodoc entry.

@CISC
Copy link
Contributor Author

CISC commented Nov 27, 2024

@gante @ArthurZucker Ready for review.

The check_code_quality workflow failure makes no sense to me, not sure how to fix?

@CISC
Copy link
Contributor Author

CISC commented Dec 3, 2024

@ArthurZucker @gante ping

@CISC
Copy link
Contributor Author

CISC commented Dec 12, 2024

@ArthurZucker ping with sugar on top?

Seems @gante is on holiday, anyone else who can review?

@Rocketknight1
Copy link
Member

Gentle ping @gante, or maybe @zucchini-nlp if I haven't totally overloaded her with pings at this point!

@CISC
Copy link
Contributor Author

CISC commented Dec 13, 2024

@Rocketknight1 Thanks! BTW, know what's up with check_code_quality and/or how to fix it?

@Rocketknight1
Copy link
Member

@CISC that's usually a code style thing. Try pip install transformers[quality] followed by make fixup

@CISC
Copy link
Contributor Author

CISC commented Dec 13, 2024

@Rocketknight1 It's not, it's complaining about the import order in src/transformers/__init__.py, which I did not change... :(

@Rocketknight1
Copy link
Member

@CISC make fixup will also resolve issues like that! In this case, the __init__.py has actually been modified in this PR to allow the class to be imported (check Files changed - it's in there!). Our code styling tools just want to correctly sort/order inputs into the standard format.

@CISC
Copy link
Contributor Author

CISC commented Dec 16, 2024

@Rocketknight1 Sure, I changed the file, but I didn't change the sort order, it should already be sorted! :P

@ArthurZucker
Copy link
Collaborator

Sorry @CISC reviewing!

Copy link
Collaborator

@ArthurZucker ArthurZucker left a comment

Choose a reason for hiding this comment

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

Looks marvelous sorry for being so late. I actually wanted to wait because we added huggingface/tokenizers#1678 a decode stream api in tokenizers that fixes some issues with prefix space, but the changes are for another PR and for the base Text streamer! 🤗 cc @gante

@ArthurZucker
Copy link
Collaborator

Do you want us to fix the CIs for you? I can push directlyu

@CISC
Copy link
Contributor Author

CISC commented Dec 20, 2024

Do you want us to fix the CIs for you? I can push directlyu

Yes please, thank you. :)

@ArthurZucker ArthurZucker merged commit eafbb0e into huggingface:main Dec 20, 2024
23 checks passed
@ArthurZucker
Copy link
Collaborator

thanks @CISC 🤗

@CISC CISC deleted the async-streamer branch December 20, 2024 11:09
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.

3 participants