ryanthompson591 commented on code in PR #17762:
URL: https://github.com/apache/beam/pull/17762#discussion_r883869791


##########
sdks/python/apache_beam/ml/inference/sklearn_inference.py:
##########
@@ -41,7 +41,9 @@ class ModelFileType(enum.Enum):
   JOBLIB = 2
 
 
-class SklearnInferenceRunner(InferenceRunner):
+class SklearnInferenceRunner(InferenceRunner[numpy.ndarray,
+                                             PredictionResult,
+                                             Any]):

Review Comment:
   yes, BaseEstimator looks right. I also checked that pipelines inherit this 
base class.



##########
sdks/python/apache_beam/ml/inference/base.py:
##########
@@ -54,17 +54,20 @@
 _NANOSECOND_TO_MICROSECOND = 1000
 _SECOND_TO_MICROSECOND = 1_000_000
 
-T = TypeVar('T')
+ModelT = TypeVar('ModelT')

Review Comment:
   I another PR Heejong is using beam.type_hints.TypeVariable.
   
   Any reason to favor one over the other?
   
   Reference PR: https://github.com/apache/beam/pull/17773



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to