search

dmlc / gluon-nlp

NLP made easy
https://nlp.gluon.ai/
Apache License 2.0
2.56k stars 538 forks source link

remove convert_all.sh #1350

Closed zheyuye closed 4 years ago

zheyuye commented 4 years ago

Description

Will fix https://github.com/dmlc/gluon-nlp/issues/1347

cc @sxjscience

szha commented 4 years ago

@sxjscience do we really need this script? I'm not sure if this is useful to users and this file seems like a place where new addition would be missed.

sxjscience commented 4 years ago

It's also fine to directly remove this file.

sxjscience commented 4 years ago

@ZheyuYe Would you remove this file?

codecov[bot] commented 4 years ago

Codecov Report

Merging #1350 into master will decrease coverage by 0.01%. The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1350      +/-   ##
==========================================
- Coverage   81.75%   81.74%   -0.02%     
==========================================
  Files          52       52              
  Lines        6862     6862              
==========================================
- Hits         5610     5609       -1     
- Misses       1252     1253       +1
Impacted Files Coverage Δ
src/gluonnlp/data/tokenizers/subword_nmt.py 78.50% <0.00%> (-0.94%) :arrow_down:
src/gluonnlp/data/tokenizers/sentencepiece.py 75.44% <0.00%> (-0.60%) :arrow_down:
src/gluonnlp/data/tokenizers/yttm.py 82.75% <0.00%> (+0.86%) :arrow_up:

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 e0b293e...95bbf17. Read the comment docs.

sxjscience commented 4 years ago

@szha Would you have other concerns?