Move preprocessing to base classes #1807
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Probably still some test breakages to work though, not mailing this out quite yet. |
mattdangerw
force-pushed
the
preprocessing-simplify
branch
2 times, most recently
from
ce944da
to
9250f79
Compare
I think this will overall be a nice simplification for maintenance. Push whatever logic we can down onto the base preprocessing classes. Saves a lot of code. To assist with this, I am adding a `special_tokens` property to tokenizers, which I think will be useful anyway.
mattdangerw
force-pushed
the
preprocessing-simplify
branch
from
9250f79
to
ea5e96c
Compare
|
Ok! Passing besides the nightly failure (which is unrelated). Mailing out. |
|
The nightly breakage is unrelated btw. |
SamanehSaadat
approved these changes
Sep 4, 2024
|
Thanks for review! Will pull this in once tests pass. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I think this will overall be a nice simplification for maintenance. Push whatever logic we can down onto the base preprocessing classes. Saves a lot of code. To assist with this, I am adding a
special_tokensproperty to tokenizers, which I think will be useful anyway.