Skip to content

Conversation

@LysandreJik
Copy link
Member

Fixes the issue detailed in #1807

Added special tokens should not be present when decoding with the skip_special_tokens flag set to True.
Using the convert_tokens_to_ids checks whether those added tokens are present and removes them if necessary.

@codecov-io
Copy link

codecov-io commented Nov 12, 2019

Codecov Report

Merging #1811 into master will decrease coverage by 0.08%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1811      +/-   ##
==========================================
- Coverage   84.16%   84.08%   -0.09%     
==========================================
  Files          94       94              
  Lines       14185    14047     -138     
==========================================
- Hits        11939    11811     -128     
+ Misses       2246     2236      -10
Impacted Files Coverage Δ
transformers/tests/tokenization_tests_commons.py 100% <100%> (ø) ⬆️
transformers/tokenization_utils.py 92.12% <100%> (+0.92%) ⬆️
transformers/modeling_tf_roberta.py 89.9% <0%> (-0.53%) ⬇️
transformers/modeling_tf_xlnet.py 87.82% <0%> (-0.37%) ⬇️
transformers/modeling_tf_transfo_xl.py 92.21% <0%> (-0.33%) ⬇️
transformers/tests/modeling_tf_common_test.py 96.8% <0%> (-0.28%) ⬇️
transformers/modeling_tf_utils.py 92.4% <0%> (-0.28%) ⬇️
transformers/modeling_tf_ctrl.py 97.75% <0%> (-0.11%) ⬇️
transformers/modeling_tf_xlm.py 90.39% <0%> (-0.08%) ⬇️
transformers/modeling_tf_distilbert.py 98.59% <0%> (-0.07%) ⬇️
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 155c782...74d0bcb. Read the comment docs.

@thomwolf
Copy link
Member

Nice fix, a lot cleaner as well

@thomwolf thomwolf merged commit 5b322a3 into master Nov 14, 2019
@julien-c julien-c deleted the special-tokens branch November 16, 2019 02:22
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.

4 participants