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

FastembedSparseTextEmbedder - remove batch_size #688

Merged
merged 1 commit into from
May 6, 2024

Conversation

anakin87
Copy link
Member

FastembedTextEmbedder can only accept and process a string at a time.
batch_size parameter is practically ignored and misleading -> I'm removing it.

(Other minor docstrings improvements)

@anakin87 anakin87 requested a review from a team as a code owner April 24, 2024 17:00
@anakin87 anakin87 requested review from shadeMe and removed request for a team April 24, 2024 17:00
@github-actions github-actions bot added integration:fastembed type:documentation Improvements or additions to documentation labels Apr 24, 2024
@shadeMe
Copy link
Contributor

shadeMe commented Apr 24, 2024

@masci Do we have a deprecation policy for core integrations? This is a breaking change.

@anakin87 anakin87 changed the title FastembedTextEmbedder - remove batch_size FastembedSparseTextEmbedder - remove batch_size Apr 24, 2024
@anakin87
Copy link
Member Author

I agree that formally this is a breaking change.

However, this component was released 2 weeks ago and it has not yet been documented or promoted.
So it should not impact many users.

In any case, let's wait for Massi's opinion.

@anakin87 anakin87 requested a review from masci April 29, 2024 08:39
@anakin87
Copy link
Member Author

anakin87 commented May 6, 2024

I talked with Massi: breaking changes are allowed in major releases.

Once this PR is merged, I will create a major release of fastembed-haystack (1.0.0).

When we have release notes for core integrations, everything will be clearer.

@anakin87 anakin87 merged commit d30c0ea into main May 6, 2024
7 checks passed
@anakin87 anakin87 deleted the fastembed-sparsetextemb-rm-batch_size branch May 6, 2024 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integration:fastembed type:documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants