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.

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

TokenTextSplitter enhancement #1558

Closed
wants to merge 3 commits into from
Closed

TokenTextSplitter enhancement #1558

wants to merge 3 commits into from

Conversation

ricken07
Copy link
Contributor

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.

spring-ai-core/src/main/java/org/springframework/ai/transformer/splitter/TokenTextSplitter.java Outdated Show resolved Hide resolved
spring-ai-core/src/main/java/org/springframework/ai/transformer/splitter/TokenTextSplitter.java Outdated Show resolved Hide resolved
spring-ai-core/src/main/java/org/springframework/ai/transformer/splitter/TokenTextSplitter.java Outdated Show resolved Hide resolved
spring-ai-core/src/main/java/org/springframework/ai/transformer/splitter/TokenTextSplitter.java Outdated Show resolved Hide resolved
spring-ai-core/src/main/java/org/springframework/ai/transformer/splitter/TokenTextSplitter.java Outdated Show resolved Hide resolved
@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

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