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
[BUG] device
should be attribute on SentenceEncoder
#33
Comments
repr
breaks on SentenceEncoder
device
should be attribute on SentenceEncoder
Feel free to make a PR. It might make sense to have the actual reference to the torch object to be |
Cool! I made PR #34 which solves this and adds some tests for As for testing text embedders it looks like |
There's no harm in adding 'self.path' I think. Feel free to make that PR as well! |
Cool! Created #35 for the Sense2Vec attribute fix. |
The
device
argument inSentenceEncoder
is not defined as an attribute. This leads to bugs when using it with sklearn. I encountered attribute errors when trying to print out aPipeline
representation that hasSentenceEncoder
as a component.Should be easy to fix by just adding
self.device
inSentenceEncoder.__init__
. We can consider adding tests for text encoders so we can catch these errors beforehand.The scikit-learn development docs make it clear every argument should be defined as an attribute:
Error message:
AttributeError: 'SentenceEncoder' object has no attribute 'device'
.Reproduction:
Python 3.8 with
embetter = "^0.2.2"
Fix:
Add
self.device
onSentenceEncoder
The text was updated successfully, but these errors were encountered: