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

Modify Processor trait to support chaining. #1047

Merged
merged 1 commit into from
Aug 24, 2022

Conversation

Narsil
Copy link
Collaborator

@Narsil Narsil commented Aug 23, 2022

0 modifications yet, everything will consume the vector.
Every test should be green without any modifications.

I wanted to resurrect #1005 but I found myself having a real hard time making modifications and understanding what was what and why things were different when they were.

Instead I decided to start from scratch heavily inspiring from the PR instead.

This PR is the first step. It modifies the trait, and that' s it.

  • Next step making the various process_encodings actually agnostic (meaning not fusing their returned results into 1 encoding when they can).
  • Then we can add Sequence.

Each step will be in a different PR.

Edit companion PR for code linting : #1048

@Narsil Narsil requested review from SaulLu, mishig25 and McPatate August 23, 2022 16:21
@HuggingFaceDocBuilderDev

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint.

0 modifications yet, everything will consume the vector.
Every test should be green without any modifications.
@Narsil Narsil force-pushed the modify_processor_trait branch from 8d16889 to c221192 Compare August 23, 2022 20:08
@Narsil Narsil changed the base branch from main to clippy_update August 23, 2022 20:08
Copy link
Contributor

@mishig25 mishig25 left a comment

Choose a reason for hiding this comment

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

lgtm!

@Narsil Narsil merged commit 1196e68 into clippy_update Aug 24, 2022
Narsil added a commit that referenced this pull request Aug 24, 2022
@SaulLu
Copy link
Contributor

SaulLu commented Aug 24, 2022

Thank you @Narsil 🙌

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.

4 participants