-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Sklearn Mnist example and IT test #21781
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
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
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
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.md
with 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.