-
Notifications
You must be signed in to change notification settings - Fork 4.5k
Sklearn Mnist example and IT test #21781
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Run Python 3.8 PostCommit |
|
Can one of the admins verify this patch? |
4 similar comments
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Run Python 3.9 PostCommit |
sdks/python/apache_beam/examples/inference/sklearn_mnist_classification.py
Outdated
Show resolved
Hide resolved
| @pytest.mark.it_postcommit | ||
| def test_predictions_output_file(self): | ||
| test_pipeline = TestPipeline(is_integration_test=True) | ||
| input_file = 'gs://apache-beam-ml/testing/inputs/it_mnist_data.csv' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if this will have to be downloaded separately, we should mention necessary instructions. You probably want to add a section in https://github.com/apache/beam/blob/master/sdks/python/apache_beam/examples/inference/README.md.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this go under apache-beam-ml/datasets/?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test would be internal test. Also sickbayed it for now. There are is PR in work #21887 on how to run the sample
sdks/python/apache_beam/ml/inference/sklearn_inference_it_test.py
Outdated
Show resolved
Hide resolved
sdks/python/apache_beam/ml/inference/sklearn_inference_it_test.py
Outdated
Show resolved
Hide resolved
yeandy
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you want me to help with writing the README?
| Tuple[int, PredictionResult]) | ||
| | "PostProcessor" >> beam.ParDo(PostProcessor())) | ||
|
|
||
| if known_args.output: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make output required. Remove if statement.
| @pytest.mark.it_postcommit | ||
| def test_predictions_output_file(self): | ||
| test_pipeline = TestPipeline(is_integration_test=True) | ||
| input_file = 'gs://apache-beam-ml/testing/inputs/it_mnist_data.csv' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this go under apache-beam-ml/datasets/?
sdks/python/apache_beam/examples/inference/sklearn_mnist_classification.py
Outdated
Show resolved
Hide resolved
| dest='input', | ||
| help='CSV file with row containing label and pixel values.') | ||
| parser.add_argument( | ||
| '--output', dest='output', help='Path to save output predictions.') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add required=True
sdks/python/apache_beam/examples/inference/sklearn_mnist_classification.py
Outdated
Show resolved
Hide resolved
8dafc57 to
fe5f829
Compare
fe5f829 to
0be2a8b
Compare
sdks/python/apache_beam/examples/inference/sklearn_mnist_classification.py
Outdated
Show resolved
Hide resolved
|
refactored the code with recent changes. Added the test but sickbayed it for now. Adding the issue here: #21859 to unskip the tests. |
Codecov Report
@@ Coverage Diff @@
## master #21781 +/- ##
==========================================
- Coverage 74.01% 74.00% -0.01%
==========================================
Files 699 700 +1
Lines 92675 92715 +40
==========================================
+ Hits 68592 68614 +22
- Misses 22828 22846 +18
Partials 1255 1255
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
cc: @yeandy Added the gradle task |
Co-authored-by: Andy Ye <andyye333@gmail.com>
|
PTAL @tvalentyn |
|
PTAL @yeandy @tvalentyn |
Co-authored-by: Andy Ye <andyye333@gmail.com>
|
Can you also uncomment pytest.skip and confirm that |
I checked and it runs. I was able to collect all the Inference IT tests |
|
@pabloem test failure unrelated to the change |
|
lgtm thanks folks |
* sklearn example and IT test * Change the example name * Refactor sklearn example * Refactor and add assertions to the sklearn test * Fixup import order * fixup: help and name * Add gradle task for sklearn IT tests * fixup lint * Update sdks/python/test-suites/direct/common.gradle Co-authored-by: Andy Ye <andyye333@gmail.com> * Change sklearn IT test marker * Uncomment * Apply suggestions from code review Co-authored-by: Andy Ye <andyye333@gmail.com> Co-authored-by: Andy Ye <andyye333@gmail.com>
* sklearn example and IT test * Change the example name * Refactor sklearn example * Refactor and add assertions to the sklearn test * Fixup import order * fixup: help and name * Add gradle task for sklearn IT tests * fixup lint * Update sdks/python/test-suites/direct/common.gradle Co-authored-by: Andy Ye <andyye333@gmail.com> * Change sklearn IT test marker * Uncomment * Apply suggestions from code review Co-authored-by: Andy Ye <andyye333@gmail.com> Co-authored-by: Andy Ye <andyye333@gmail.com>
Scikit learn example that runs on MNIST data. An IT test that runs and asserts the output on subset of MNIST data.
Also, a pytest marker that collects all the Inference IT tests and run them in them PostCommit suite on Direct Runner.
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
R: @username).CHANGES.mdwith noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.
To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md
GitHub Actions Tests Status (on master branch)
See CI.md for more information about GitHub Actions CI.