Another multilingual approach #177
Conversation
richard-rogers
force-pushed
the
dev/richard/multilingual3
branch
2 times, most recently
from
dd6269b
to
824c027
Compare
langkit/sentiment.py
Outdated
|
|
||
| def sentiment_nltk(text: str) -> float: | ||
| if _sentiment_analyzer is None: | ||
| def sentiment_nltk(text: str, sentiment_analyzer) -> float: |
There was a problem hiding this comment.
I think we should continue supporting the use of the modules as standalone functions. It looks like this change would prevent the user from simply calling sentiment_nltk("I am very sad"), right?
I think this comment applies to other modules. At least for toxicity, I think it's the same case.
Can we require only the text values for the standalone functions?
There was a problem hiding this comment.
I don't see a way to have a single function support multiple languages without the extra parameter. There's a few ways it could be curried, but logically it needs to be there.
There was a problem hiding this comment.
I think having an extra parameter is ok, but requiring the user to pass a SentimentIntensityAnalyzer, for example, looks very complex. Couldn't we have the user be able to pass an optional language parameter that defaults to english?
we can choose to drop the single function support, but I personally find it very useful.
| pass | ||
|
|
||
|
|
||
| init() |
There was a problem hiding this comment.
The removal of the init() call in the import of these modules: (textstat, sentiment, regexes, etc) is a significant breaking change in behavior and will break existing LangKit integrations that imported these modules directly.
Can we provide a back-compat friendly version of the language support changes?
richard-rogers
force-pushed
the
dev/richard/multilingual3
branch
from
ef52f92
to
17fb02b
Compare
richard-rogers
force-pushed
the
dev/richard/multilingual3
branch
from
17fb02b
to
bf99652
Compare
init()on import!Translatorinterface for polyglotting English-only metrics