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

verbosity and CharacterTextSplitter #35

Merged
merged 8 commits into from
Oct 18, 2023
Merged

verbosity and CharacterTextSplitter #35

merged 8 commits into from
Oct 18, 2023

Conversation

TesAnti
Copy link
Collaborator

@TesAnti TesAnti commented Oct 18, 2023

see respective commits comments for the details.

Note: sorry if i'm doing something wrong. This is my first contribution to opensource project

Closes #12

TesAnti and others added 4 commits October 17, 2023 23:34
…ning only ["text'] key ignoring OutputKey. This was causing an error when used with SequentialChain
- had to create and use TextSplitterBase class and Document class to match structure of original python implementation
- i have not implemented methods which are relying on external source of information(from_huggingface_tokenizer,from_tiktoken_encoder)
- added comments which are explaining logic behind implementation. it should help new people to understand what it does(original python implementation kind of hard to understand in some moments)
@HavenDV
Copy link
Contributor

HavenDV commented Oct 18, 2023

Perfect! It was fast and looks good

I see only one point that causes a warning and in general is rarely used in C# - delegates. Just use Func<string, int> instead

@TesAnti
Copy link
Collaborator Author

TesAnti commented Oct 18, 2023

Removed the delegate

@HavenDV HavenDV merged commit 680c9f4 into tryAGI:main Oct 18, 2023
1 check passed
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.

CharacterTextSplitter
2 participants