You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2022/06/10 18:57:26 UTC
[GitHub] [beam] AnandInguva opened a new pull request, #21801: Refactor API code to base.py in RunInference
AnandInguva opened a new pull request, #21801:
URL: https://github.com/apache/beam/pull/21801
**Please** add a meaningful description for your change here
------------------------
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
- [ ] [**Choose reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and mention them in a comment (`R: @username`).
- [ ] Mention the appropriate issue in your description (for example: "addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, comment "fixes #<ISSUE NUMBER>" instead.
- [ ] Update `CHANGES.md` with noteworthy changes.
- [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://beam.apache.org/contribute/#make-reviewers-job-easier).
To check the build health, please visit [https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md](https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md)
GitHub Actions Tests Status (on master branch)
------------------------------------------------------------------------------------------------
[](https://github.com/apache/beam/actions?query=workflow%3A%22Build+python+source+distribution+and+wheels%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Python+Tests%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Java+Tests%22+branch%3Amaster+event%3Aschedule)
See [CI.md](https://github.com/apache/beam/blob/master/CI.md) for more information about GitHub Actions CI.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] AnandInguva commented on a diff in pull request #21801: Refactor API code to base.py in RunInference
Posted by GitBox <gi...@apache.org>.
AnandInguva commented on code in PR #21801:
URL: https://github.com/apache/beam/pull/21801#discussion_r895929839
##########
sdks/python/apache_beam/ml/inference/base.py:
##########
@@ -14,6 +14,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.
#
+# mypy: ignore-errors
Review Comment:
```
pache_beam/testing/load_tests/load_test_metrics_utils.py:51: error: unused 'type: ignore' comment
apache_beam/ml/inference/base.py:68: error: Type variable "apache_beam.ml.inference.base._INPUT_TYPE" is unbound [valid-type]
apache_beam/ml/inference/base.py:68: note: (Hint: Use "Generic[_INPUT_TYPE]" or "Protocol[_INPUT_TYPE]" base class to bind "_INPUT_TYPE" inside a class)
apache_beam/ml/inference/base.py:68: note: (Hint: Use "_INPUT_TYPE" in function signature to bind "_INPUT_TYPE" inside a function)
apache_beam/ml/inference/base.py:69: error: Type variable "apache_beam.ml.inference.base._OUTPUT_TYPE" is unbound [valid-type]
apache_beam/ml/inference/base.py:69: note: (Hint: Use "Generic[_OUTPUT_TYPE]" or "Protocol[_OUTPUT_TYPE]" base class to bind "_OUTPUT_TYPE" inside a class)
apache_beam/ml/inference/base.py:69: note: (Hint: Use "_OUTPUT_TYPE" in function signature to bind "_OUTPUT_TYPE" inside a function
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] AnandInguva commented on a diff in pull request #21801: Refactor API code to base.py in RunInference
Posted by GitBox <gi...@apache.org>.
AnandInguva commented on code in PR #21801:
URL: https://github.com/apache/beam/pull/21801#discussion_r895936744
##########
sdks/python/apache_beam/ml/inference/base.py:
##########
@@ -14,6 +14,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.
#
+# mypy: ignore-errors
Review Comment:
Find an issue related to this. https://github.com/python/mypy/issues/7520.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] tvalentyn commented on a diff in pull request #21801: Refactor API code to base.py in RunInference
Posted by GitBox <gi...@apache.org>.
tvalentyn commented on code in PR #21801:
URL: https://github.com/apache/beam/pull/21801#discussion_r895956858
##########
sdks/python/apache_beam/ml/inference/base.py:
##########
@@ -206,6 +216,19 @@ class RunInference(beam.PTransform[beam.PCollection[ExampleT],
Args:
model_handler: An implementation of ModelHandler.
clock: A clock implementing get_current_time_in_microseconds.
+
+ A transform that takes a PCollection of examples (or features) to be used on
+ an ML model. It will then output inferences (or predictions) for those
+ examples in a PCollection of PredictionResults, containing the input examples
+ and output inferences.
+
+ If examples are paired with keys, it will output a tuple
+ (key, PredictionResult) for each (key, example) input.
+
+ Models for supported frameworks can be loaded via a URI. Supported services
Review Comment:
TODO in a separate PR:
- proof-read these API bits by TW.
- Supported services can also be used - what is this about?
- TODO(BEAM-14046): Add and link to help documentation : Let's point to the examples directory.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] AnandInguva commented on pull request #21801: Refactor API code to base.py in RunInference
Posted by GitBox <gi...@apache.org>.
AnandInguva commented on PR #21801:
URL: https://github.com/apache/beam/pull/21801#issuecomment-1152670188
R: @robertwb @ryanthompson591
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] AnandInguva commented on a diff in pull request #21801: Refactor API code to base.py in RunInference
Posted by GitBox <gi...@apache.org>.
AnandInguva commented on code in PR #21801:
URL: https://github.com/apache/beam/pull/21801#discussion_r895677372
##########
sdks/python/apache_beam/ml/inference/base.py:
##########
@@ -14,6 +14,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.
#
+# mypy: ignore-errors
Review Comment:
it was addressed in this #17514 and commented on this issue #21441. I added a todo( link to github issue)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] asf-ci commented on pull request #21801: Refactor API code to base.py in RunInference
Posted by GitBox <gi...@apache.org>.
asf-ci commented on PR #21801:
URL: https://github.com/apache/beam/pull/21801#issuecomment-1152666498
Can one of the admins verify this patch?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] AnandInguva commented on a diff in pull request #21801: Refactor API code to base.py in RunInference
Posted by GitBox <gi...@apache.org>.
AnandInguva commented on code in PR #21801:
URL: https://github.com/apache/beam/pull/21801#discussion_r895945139
##########
sdks/python/apache_beam/ml/inference/base.py:
##########
@@ -14,6 +14,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.
#
+# mypy: ignore-errors
Review Comment:
Looking into these errors, this is more related to mypy and Dataclass[1]. Since we are moving to use `NamedTuple` instead of `Dataclass`, can we move forward with this?
[1] https://github.com/python/mypy/issues/7520. To solve the current error of mypy with dataclass, PredictionResult should be modified as below
```
@dataclass
class PredictionResult(Generic[_INPUT_TYPE, _OUTPUT_TYPE]):
...
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] asf-ci commented on pull request #21801: Refactor API code to base.py in RunInference
Posted by GitBox <gi...@apache.org>.
asf-ci commented on PR #21801:
URL: https://github.com/apache/beam/pull/21801#issuecomment-1152665676
Can one of the admins verify this patch?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] AnandInguva commented on a diff in pull request #21801: Refactor API code to base.py in RunInference
Posted by GitBox <gi...@apache.org>.
AnandInguva commented on code in PR #21801:
URL: https://github.com/apache/beam/pull/21801#discussion_r895961267
##########
sdks/python/apache_beam/ml/inference/base.py:
##########
@@ -206,6 +216,19 @@ class RunInference(beam.PTransform[beam.PCollection[ExampleT],
Args:
model_handler: An implementation of ModelHandler.
clock: A clock implementing get_current_time_in_microseconds.
+
+ A transform that takes a PCollection of examples (or features) to be used on
+ an ML model. It will then output inferences (or predictions) for those
+ examples in a PCollection of PredictionResults, containing the input examples
+ and output inferences.
+
+ If examples are paired with keys, it will output a tuple
+ (key, PredictionResult) for each (key, example) input.
+
+ Models for supported frameworks can be loaded via a URI. Supported services
Review Comment:
Yep. thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] ihji commented on pull request #21801: Refactor API code to base.py in RunInference
Posted by GitBox <gi...@apache.org>.
ihji commented on PR #21801:
URL: https://github.com/apache/beam/pull/21801#issuecomment-1152793765
> Looks good. @ihji this will probably change what you do with your PR.
Got it. Will update my PR accordingly after this PR gets merged.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] asf-ci commented on pull request #21801: Refactor API code to base.py in RunInference
Posted by GitBox <gi...@apache.org>.
asf-ci commented on PR #21801:
URL: https://github.com/apache/beam/pull/21801#issuecomment-1152665677
Can one of the admins verify this patch?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] yeandy commented on a diff in pull request #21801: Refactor API code to base.py in RunInference
Posted by GitBox <gi...@apache.org>.
yeandy commented on code in PR #21801:
URL: https://github.com/apache/beam/pull/21801#discussion_r894857862
##########
sdks/python/apache_beam/ml/inference/base.py:
##########
@@ -14,6 +14,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.
#
+# mypy: ignore-errors
Review Comment:
What was the error here? was it something like `Type variable is unbound`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] asf-ci commented on pull request #21801: Refactor API code to base.py in RunInference
Posted by GitBox <gi...@apache.org>.
asf-ci commented on PR #21801:
URL: https://github.com/apache/beam/pull/21801#issuecomment-1152664981
Can one of the admins verify this patch?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] tvalentyn commented on pull request #21801: Refactor API code to base.py in RunInference
Posted by GitBox <gi...@apache.org>.
tvalentyn commented on PR #21801:
URL: https://github.com/apache/beam/pull/21801#issuecomment-1154182311
merging based on results from previous commit run.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] tvalentyn merged pull request #21801: Refactor API code to base.py in RunInference
Posted by GitBox <gi...@apache.org>.
tvalentyn merged PR #21801:
URL: https://github.com/apache/beam/pull/21801
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] AnandInguva commented on a diff in pull request #21801: Refactor API code to base.py in RunInference
Posted by GitBox <gi...@apache.org>.
AnandInguva commented on code in PR #21801:
URL: https://github.com/apache/beam/pull/21801#discussion_r895929839
##########
sdks/python/apache_beam/ml/inference/base.py:
##########
@@ -14,6 +14,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.
#
+# mypy: ignore-errors
Review Comment:
These are the errors mypy complains about.
```
apache_beam/testing/load_tests/load_test_metrics_utils.py:51: error: unused 'type: ignore' comment
apache_beam/ml/inference/base.py:68: error: Type variable "apache_beam.ml.inference.base._INPUT_TYPE" is unbound [valid-type]
apache_beam/ml/inference/base.py:68: note: (Hint: Use "Generic[_INPUT_TYPE]" or "Protocol[_INPUT_TYPE]" base class to bind "_INPUT_TYPE" inside a class)
apache_beam/ml/inference/base.py:68: note: (Hint: Use "_INPUT_TYPE" in function signature to bind "_INPUT_TYPE" inside a function)
apache_beam/ml/inference/base.py:69: error: Type variable "apache_beam.ml.inference.base._OUTPUT_TYPE" is unbound [valid-type]
apache_beam/ml/inference/base.py:69: note: (Hint: Use "Generic[_OUTPUT_TYPE]" or "Protocol[_OUTPUT_TYPE]" base class to bind "_OUTPUT_TYPE" inside a class)
apache_beam/ml/inference/base.py:69: note: (Hint: Use "_OUTPUT_TYPE" in function signature to bind "_OUTPUT_TYPE" inside a function
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] tvalentyn commented on a diff in pull request #21801: Refactor API code to base.py in RunInference
Posted by GitBox <gi...@apache.org>.
tvalentyn commented on code in PR #21801:
URL: https://github.com/apache/beam/pull/21801#discussion_r895921320
##########
sdks/python/apache_beam/ml/inference/base.py:
##########
@@ -14,6 +14,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.
#
+# mypy: ignore-errors
Review Comment:
I think #21441 describes a different problem: supporting type inference for RunInference API users. I believe some of the approaches, that it mentions, have been implemented (adding generics), and we have some type inference capabilities (that you plan to test in another test). I'd ask, what is the remaining work there?
The problem of having to disable mypy in this file seems to do with developer tooling. Do we know what causes mypy to complain? Does this problem need a separate issue?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] AnandInguva commented on a diff in pull request #21801: Refactor API code to base.py in RunInference
Posted by GitBox <gi...@apache.org>.
AnandInguva commented on code in PR #21801:
URL: https://github.com/apache/beam/pull/21801#discussion_r895945139
##########
sdks/python/apache_beam/ml/inference/base.py:
##########
@@ -14,6 +14,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.
#
+# mypy: ignore-errors
Review Comment:
Looking into these errors, this is more related to mypy and Dataclass[1]. Since we are moving to use `NamedTuple` instead of `Dataclass`, can we move forward with this?
[1] https://github.com/python/mypy/issues/7520
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] tvalentyn commented on a diff in pull request #21801: Refactor API code to base.py in RunInference
Posted by GitBox <gi...@apache.org>.
tvalentyn commented on code in PR #21801:
URL: https://github.com/apache/beam/pull/21801#discussion_r895641916
##########
sdks/python/apache_beam/ml/inference/base.py:
##########
@@ -14,6 +14,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.
#
+# mypy: ignore-errors
Review Comment:
Do we understand why we hit that error?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] tvalentyn commented on a diff in pull request #21801: Refactor API code to base.py in RunInference
Posted by GitBox <gi...@apache.org>.
tvalentyn commented on code in PR #21801:
URL: https://github.com/apache/beam/pull/21801#discussion_r895921320
##########
sdks/python/apache_beam/ml/inference/base.py:
##########
@@ -14,6 +14,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.
#
+# mypy: ignore-errors
Review Comment:
I think #21441 describes a different problem: supporting type inference for RunInference API users. I believe some of the approaches, that it mentions, have been implemented (adding generics), and we have some type inference capabilities (that you plan to test in another test). I'd ask, what is the remaining work there?
The problem of having to disable mypy in this file seems to do with developer tooling. Do we know what causes mypy to complain? Does this problem need a separate Jira?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] asf-ci commented on pull request #21801: Refactor API code to base.py in RunInference
Posted by GitBox <gi...@apache.org>.
asf-ci commented on PR #21801:
URL: https://github.com/apache/beam/pull/21801#issuecomment-1152664978
Can one of the admins verify this patch?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] AnandInguva commented on a diff in pull request #21801: Refactor API code to base.py in RunInference
Posted by GitBox <gi...@apache.org>.
AnandInguva commented on code in PR #21801:
URL: https://github.com/apache/beam/pull/21801#discussion_r894839722
##########
sdks/python/apache_beam/ml/inference/sklearn_inference_test.py:
##########
@@ -134,9 +133,9 @@ def test_predict_output(self):
numpy.array([1, 2, 3]), numpy.array([4, 5, 6]), numpy.array([7, 8, 9])
]
expected_predictions = [
- api.PredictionResult(numpy.array([1, 2, 3]), 6),
- api.PredictionResult(numpy.array([4, 5, 6]), 15),
- api.PredictionResult(numpy.array([7, 8, 9]), 24)
+ base.PredictionResult(numpy.array([1, 2, 3]), 6),
Review Comment:
yep. we can do that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] AnandInguva commented on a diff in pull request #21801: Refactor API code to base.py in RunInference
Posted by GitBox <gi...@apache.org>.
AnandInguva commented on code in PR #21801:
URL: https://github.com/apache/beam/pull/21801#discussion_r895929586
##########
sdks/python/apache_beam/ml/inference/base.py:
##########
@@ -14,6 +14,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.
#
+# mypy: ignore-errors
Review Comment:
`pache_beam/testing/load_tests/load_test_metrics_utils.py:51: error: unused 'type: ignore' comment
apache_beam/ml/inference/base.py:68: error: Type variable "apache_beam.ml.inference.base._INPUT_TYPE" is unbound [valid-type]
apache_beam/ml/inference/base.py:68: note: (Hint: Use "Generic[_INPUT_TYPE]" or "Protocol[_INPUT_TYPE]" base class to bind "_INPUT_TYPE" inside a class)
apache_beam/ml/inference/base.py:68: note: (Hint: Use "_INPUT_TYPE" in function signature to bind "_INPUT_TYPE" inside a function)
apache_beam/ml/inference/base.py:69: error: Type variable "apache_beam.ml.inference.base._OUTPUT_TYPE" is unbound [valid-type]
apache_beam/ml/inference/base.py:69: note: (Hint: Use "Generic[_OUTPUT_TYPE]" or "Protocol[_OUTPUT_TYPE]" base class to bind "_OUTPUT_TYPE" inside a class)
apache_beam/ml/inference/base.py:69: note: (Hint: Use "_OUTPUT_TYPE" in function signature to bind "_OUTPUT_TYPE" inside a function`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] AnandInguva commented on pull request #21801: Refactor API code to base.py in RunInference
Posted by GitBox <gi...@apache.org>.
AnandInguva commented on PR #21801:
URL: https://github.com/apache/beam/pull/21801#issuecomment-1154080135
PTAL @tvalentyn
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] ryanthompson591 commented on a diff in pull request #21801: Refactor API code to base.py in RunInference
Posted by GitBox <gi...@apache.org>.
ryanthompson591 commented on code in PR #21801:
URL: https://github.com/apache/beam/pull/21801#discussion_r894836860
##########
sdks/python/apache_beam/ml/inference/sklearn_inference_test.py:
##########
@@ -134,9 +133,9 @@ def test_predict_output(self):
numpy.array([1, 2, 3]), numpy.array([4, 5, 6]), numpy.array([7, 8, 9])
]
expected_predictions = [
- api.PredictionResult(numpy.array([1, 2, 3]), 6),
- api.PredictionResult(numpy.array([4, 5, 6]), 15),
- api.PredictionResult(numpy.array([7, 8, 9]), 24)
+ base.PredictionResult(numpy.array([1, 2, 3]), 6),
Review Comment:
WDYT of just importing PredictionResult directly like we do in the pytorch test.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] AnandInguva commented on a diff in pull request #21801: Refactor API code to base.py in RunInference
Posted by GitBox <gi...@apache.org>.
AnandInguva commented on code in PR #21801:
URL: https://github.com/apache/beam/pull/21801#discussion_r894841248
##########
sdks/python/apache_beam/ml/inference/sklearn_inference_test.py:
##########
@@ -134,9 +133,9 @@ def test_predict_output(self):
numpy.array([1, 2, 3]), numpy.array([4, 5, 6]), numpy.array([7, 8, 9])
]
expected_predictions = [
- api.PredictionResult(numpy.array([1, 2, 3]), 6),
- api.PredictionResult(numpy.array([4, 5, 6]), 15),
- api.PredictionResult(numpy.array([7, 8, 9]), 24)
+ base.PredictionResult(numpy.array([1, 2, 3]), 6),
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] AnandInguva commented on a diff in pull request #21801: Refactor API code to base.py in RunInference
Posted by GitBox <gi...@apache.org>.
AnandInguva commented on code in PR #21801:
URL: https://github.com/apache/beam/pull/21801#discussion_r894858325
##########
sdks/python/apache_beam/ml/inference/base.py:
##########
@@ -14,6 +14,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.
#
+# mypy: ignore-errors
Review Comment:
Yes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] AnandInguva commented on a diff in pull request #21801: Refactor API code to base.py in RunInference
Posted by GitBox <gi...@apache.org>.
AnandInguva commented on code in PR #21801:
URL: https://github.com/apache/beam/pull/21801#discussion_r895945139
##########
sdks/python/apache_beam/ml/inference/base.py:
##########
@@ -14,6 +14,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.
#
+# mypy: ignore-errors
Review Comment:
Looking into these errors, this is more related to mypy and Dataclass[1]. Since we are moving to use `NamedTuple` instead of `Dataclass`, can we move forward with this?
[1] https://github.com/python/mypy/issues/7520. To solve the current error of mypy with dataclass, PredictionResult should be modified as below
```
class PredictionResult(Generic[_INPUT_TYPE, _OUTPUT_TYPE]):
...
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org