Update load_sp_model func to accept a file object #965
Conversation
Codecov Report
@@ Coverage Diff @@
## master #965 +/- ##
=======================================
Coverage 77.53% 77.54%
=======================================
Files 44 44
Lines 3085 3090 +5
=======================================
+ Hits 2392 2396 +4
- Misses 693 694 +1
Continue to review full report at Codecov.
|
|
@hudeven Let me know if this API works for the internal cases. |
| elif isinstance(spm, io.BufferedReader): | ||
| return torch.ops.torchtext.load_sp_model_string(spm.read()) | ||
| else: | ||
| raise RuntimeError('the input of the load_sp_model func is not supported.') |
There was a problem hiding this comment.
@zhangguanheng66 PathManager.open() returns "_io.TextIOWrapper" type and will fail here. How about remove the check?
| elif isinstance(spm, io.BufferedReader): | |
| return torch.ops.torchtext.load_sp_model_string(spm.read()) | |
| else: | |
| raise RuntimeError('the input of the load_sp_model func is not supported.') | |
| return torch.ops.torchtext.load_sp_model_string(spm.read()) |
There was a problem hiding this comment.
I notice that and was thinking about adding that check.
However, with PathManager.open() , it will return f as _io.TextIOWrapper. It seems not working with sentencepiece constructor. It needs to be binary reading where PathManager.open(, 'rb') returns io.BufferedReader. @hudeven
There was a problem hiding this comment.
@zhangguanheng66 you are right! It LGTM. Note: there is a test failure
| elif isinstance(spm, io.BufferedReader): | ||
| return torch.ops.torchtext.load_sp_model_string(spm.read()) | ||
| else: | ||
| raise RuntimeError('the input of the load_sp_model func is not supported.') |
There was a problem hiding this comment.
If the type is not detect you can specify the type found in the error message to make it easier to debug.
| @@ -68,7 +68,7 @@ def test_sentencepiece_numericalizer(self): | |||
| def test_sentencepiece_tokenizer(self): | |||
| test_sample = 'SentencePiece is an unsupervised text tokenizer and detokenizer' | |||
| model_path = get_asset_path('spm_example.model') | |||
| sp_model = load_sp_model(model_path) | |||
| sp_model = load_sp_model(open(model_path, 'rb')) | |||
There was a problem hiding this comment.
assertRaises an unsupported type
Test against
pytext.utils.file_io.PathManagerand it works withspm = load_sp_model(PathManager.open('m_user.model', 'rb'))