-
Notifications
You must be signed in to change notification settings - Fork 826
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
Fix encode_batch and encode_batch_fast to accept ndarrays again #1679
Conversation
@ArthurZucker mind giving this a look since I think sooner than later someone is going to complain about |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM would be nice to add a case with ndarrays in the test encode format 🤗 LGTM otherwise, good catch
(Sorry about the delay we were on a company wide offsite 😅 🌴 |
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
Could you update with clippy as well? 🤗 |
@@ -152,8 +152,6 @@ def test_encode(self): | |||
assert len(output) == 2 | |||
|
|||
def test_encode_formats(self, bert_files): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ArthurZucker there are already a set of tests covering np.array
- did you have something else in mind?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope I checked and found then so approved and good to go!
Thanks! 🤗 |
* Fix encode_batch and encode_batch_fast to accept ndarrays again * Fix clippy --------- Co-authored-by: Dimitris Iliopoulos <diliopoulos@fb.com>
This is a follow up to my comment from here which essentially reverts the
PyList
change from #1665 andPySequence
change from #1673 with regards to theinput
arg forencode_batch
andencode_batch_fast
, back toVec<..>
. This allows passingndarray
together withlist
andtuple
asinput
types. I also turned on the tests that were turned off before to make sure that this change forencode_batch
is covered. I will follow up with adding more tests forencode_batch_fast
but prefer to get this out sooner than later.