Skip to content

TokenTextSplitter enhancement #1558

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 3 commits into from
Closed

TokenTextSplitter enhancement #1558

wants to merge 3 commits into from

Conversation

@ricken07
Copy link
Contributor

@ricken07 ricken07 commented Oct 18, 2024

This PR contains an enhancement to the splitter, adding the ability to compose the various parameters and make them immutable.

tzolov
tzolov reviewed Oct 22, 2024
View reviewed changes
Copy link
Contributor

@tzolov tzolov left a comment

Choose a reason for hiding this comment

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

Thans @ricken07
Builder will improve the usability.
I've left few adjustment suggestions.

Also can you add a builder test to the TokenTextSplitterTest as well.

@tzolov tzolov self-assigned this Oct 22, 2024
@tzolov tzolov added this to the 1.0.0-M4 milestone Oct 22, 2024
@tzolov tzolov added the ETL label Oct 22, 2024
@ricken07
Copy link
Contributor Author

ricken07 commented Oct 22, 2024

Thanks, @tzolov
Your suggestions have been taken into account and a test class has been added.

@tzolov
Copy link
Contributor

tzolov commented Oct 22, 2024

Thank you @ricken07
Rebased, squashed and merged at 745e718

@tzolov tzolov closed this Oct 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tzolov tzolov tzolov left review comments

Assignees

@tzolov tzolov

Labels

ETL

Projects

None yet

Milestone

1.0.0-M4

Development

Successfully merging this pull request may close these issues.

2 participants

@ricken07 @tzolov