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

Add fasttext #92

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

thelittlefireman
Copy link

@thelittlefireman thelittlefireman commented May 6, 2024

#70

fix GMT WARN org.languagetool.language.identifier.DefaultLanguageIdentifier fastText not configured - language detection performance will be degraded. See https://dev.languagetool.org/http-server#starting-from-command-line for instructions.

@thelittlefireman
Copy link
Author

Fix snyk ?

@thelittlefireman
Copy link
Author

Up

@Erikvl87
Copy link
Owner

Erikvl87 commented Sep 17, 2024

Hi @thelittlefireman, thank you for your contribution and the effort you've put into integrating FastText. I can see how this addition could enhance the functionality for a lot of users, however, I have some concerns regarding this integration:

Image Size and Cleanliness: Adding FastText directly to the base image significantly increases its size. One of my goals is to keep the base image as clean and lightweight as possible to ensure fast deployment and scalability.

Model Flexibility: By directly including FastText, we limit users to a specific model and implementation. I believe there are multiple FastText packages and models available, and I'd prefer to provide users with the flexibility to choose or integrate the model that best suits their needs.

I’m inclined to reject this PR in its current form to maintain the simplicity and flexibility of the base image. However, I'm open to exploring alternative solutions that address these concerns. Perhaps we can look into making FastText support an optional component?

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.

2 participants