Conversation
ArthurZucker
left a comment
ArthurZucker
left a comment
There was a problem hiding this comment.
Overall looks good to me, not sure how much of an in depth review is needed, but decode is kind of a must, we can have streaming for a later use case.
Parallel would be nice, not sure how hard it is to pull it off
| HF_TOKEN: ${{ secrets.HF_TOKEN }} | ||
| run: uvx --from huggingface_hub hf download hf-internal-testing/tokenizers-test-data gpt2.json --repo-type dataset --revision e20ac5914f5ada06874e4f9d45e49b31a049904e --local-dir ../../tokenizers/data | ||
|
|
||
| # examples/Makefile's `test` target builds and runs encode.c and null_safety.c against |
There was a problem hiding this comment.
I don't understand much of this 馃槗 but yeah, we don't want to run too many test, just test the bindings (test we can encode, we can decode, we can load, but not test results in a whole bunch of stuff)
| /// 2. `out` must be valid, writable pointer to a [`TkHandle<TkTokenizer>`]. On return, it | ||
| /// holds a live handle if this function returns NULL, or NULL otherwise. | ||
| #[unsafe(no_mangle)] | ||
| pub unsafe extern "C" fn tk_tokenizer_from_file( |
There was a problem hiding this comment.
from pretrained would be nice is well, but fine without
There was a problem hiding this comment.
my only comment is to align as much as possible with the name of the function we have for node when relevant. Same for the structure names and etc?
ee36db9 to
3c72e8e
Compare
b104398 to
2869f73
Compare
2 participants
No description provided.