Added Integration test for Fasttext #1221
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1221 +/- ##
=======================================
Coverage 65.98% 65.98%
=======================================
Files 135 135
Lines 8563 8563
=======================================
Hits 5650 5650
Misses 2913 2913 Continue to review full report at Codecov.
|
|
@flosincapite @parano Please review my pull request. |
| class FasttextClassifier(bentoml.BentoService): | ||
| @bentoml.api(input=JsonInput(), batch=False) | ||
| def predict(self, parsed_json): | ||
| # parsed_json is of type str - make it a dict |
There was a problem hiding this comment.
Check out this similar test implementation and the corresponding test. I think something else is causing parsed_json to be a string.
| return FasttextClassifier | ||
|
|
||
|
|
||
| test_json = json.dumps({"text": "foo"}) |
There was a problem hiding this comment.
Ahhh. I think if you just omit json.dumps here, you should be fine :)
There was a problem hiding this comment.
Thanks for catching that! I've made the changes accordingly!.
| temporary_file = tempfile.NamedTemporaryFile(suffix=".txt", mode="w+") | ||
| temporary_file.write("__label__bar foo") | ||
| # Set file pointer to begging to ensure correct read | ||
| temporary_file.seek(0) |
There was a problem hiding this comment.
You can just close the temporary file here--no need for seek.
There was a problem hiding this comment.
The temporary file ends up being automatically destroyed once it is closed.
Would you suggest using a function such as mkstemp?
File deletion would have to be handled explicitly in that case!
There was a problem hiding this comment.
I lied! I assumed tempfile worked like mkstemp, but it doesn't. You did the right thing.
| temporary_file = tempfile.NamedTemporaryFile(suffix=".txt", mode="w+") | ||
| temporary_file.write("__label__bar foo") | ||
| # Set file pointer to begging to ensure correct read | ||
| temporary_file.seek(0) |
There was a problem hiding this comment.
I lied! I assumed tempfile worked like mkstemp, but it doesn't. You did the right thing.
telescopic
force-pushed
the
fasttext-tests
branch
from
590ab84
to
16639b1
Compare
| # Set file pointer to beginning to ensure correct read | ||
| temporary_file.seek(0) | ||
| yield temporary_file.name | ||
| temporary_file.close() |
There was a problem hiding this comment.
Yeahhhhh loving this contextmanager version!
* Add Fasttext Classifier class * Add Fasttext integration test * Add .sh file for fasttext integration test
Description
This PR adds an integration test for the Fasttext artifact code.
Changes:
Motivation and Context
This change is required as Fasttext artifact code is currently not tested.
This PR solves #1195
How Has This Been Tested?
Types of changes
Component(s) if applicable
Checklist:
./dev/format.shand./dev/lint.shscript have passed(instructions).