[GitHub] [beam] yeandy commented on a diff in pull request #21810: Split PytorchModelHandler into PytorchModelHandlerTensor and PytorchModelHandlerKeyedTensor

[GitBox <gi...@apache.org>]

yeandy commented on code in PR #21810:
URL: https://github.com/apache/beam/pull/21810#discussion_r894896292


##########
sdks/python/apache_beam/ml/inference/pytorch_inference.py:
##########
@@ -29,6 +29,7 @@
 from apache_beam.io.filesystems import FileSystems
 from apache_beam.ml.inference.api import PredictionResult
 from apache_beam.ml.inference.base import ModelHandler
+from apache_beam.utils.annotations import experimental
 
 
 class PytorchModelHandler(ModelHandler[torch.Tensor,

Review Comment:
   Is the intention to still keep `PytorchModelHandler`?



##########
sdks/python/apache_beam/ml/inference/pytorch_inference_test.py:
##########
@@ -38,6 +38,8 @@
   from apache_beam.ml.inference.api import PredictionResult
   from apache_beam.ml.inference.base import RunInference
   from apache_beam.ml.inference.pytorch_inference import PytorchModelHandler

Review Comment:
   For the tests, I changed them to use PytorchModelHandlerTensor, and PytorchModelHandlerKeyedTensor. Should the tests default to use `PytorchModelHandler` since PytorchModelHandlerTensor, and PytorchModelHandlerKeyedTensor are experimental? How should we handle this



-- 
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