gemini-code-assist[bot] commented on code in PR #35716:
URL: https://github.com/apache/beam/pull/35716#discussion_r2237259988
##########
sdks/python/apache_beam/ml/transforms/embeddings/open_ai.py:
##########
@@ -71,6 +75,12 @@ def __init__(
user: Optional[str] = None,
max_batch_size: Optional[int] = None,
):
+ if not openai:
+ raise ImportError(
+ "OpenAI Python SDK is required to use "
+ "OpenAITextEmbeddings. "
+ "Please install it with using `pip install openai`.")
Review Comment:
The phrase 'with using' is slightly ungrammatical. 'using' is sufficient and
more natural here.
```suggestion
"Please install it using `pip install openai`.")
```
##########
sdks/python/apache_beam/ml/transforms/embeddings/huggingface.py:
##########
@@ -71,7 +71,7 @@ def __init__(
if not SentenceTransformer:
raise ImportError(
"sentence-transformers is required to use "
- "SentenceTransformerEmbeddings."
+ "SentenceTransformerEmbeddings. "
"Please install it with using `pip install sentence-transformers`.")
Review Comment:
The phrase 'with using' is slightly ungrammatical. 'using' is sufficient and
more natural here.
```suggestion
"Please install it using `pip install sentence-transformers`.")
```
##########
sdks/python/apache_beam/yaml/yaml_ml.py:
##########
@@ -37,6 +41,17 @@
except ImportError:
tft = None # type: ignore
+# Iterate over all submodules in the 'embeddings' package path
+for module_info in pkgutil.iter_modules(embeddings.__path__,
+ f"{embeddings.__name__}."):
+ try:
+ submodule = importlib.import_module(module_info.name)
+ for name in getattr(submodule, '__all__', []):
+ _transform_constructors[name] = getattr(submodule, name)
+ except ImportError:
+ # Skip any modules that fail to import
+ continue
Review Comment:
Silently ignoring `ImportError` can make debugging difficult if a transform
is unexpectedly missing. For example, a typo in an import statement within one
of the embedding modules would be silenced. It would be better to log these
errors to provide visibility into why a particular transform might not be
available.
```suggestion
except ImportError as e:
# Skip any modules that fail to import, but log a warning.
logging.warning('Could not import %s, skipping: %s', module_info.name, e)
continue
```
##########
sdks/python/apache_beam/ml/transforms/embeddings/vertex_ai.py:
##########
@@ -89,6 +93,13 @@ def __init__(
location: Optional[str] = None,
credentials: Optional[Credentials] = None,
**kwargs):
+ if not vertexai:
+ raise ImportError(
+ "Vertex AI Python SDK is required to use "
+ "VertexAITextEmbeddings and VertexAIImageEmbeddings. "
+ "Please install it with using "
Review Comment:
The phrase 'with using' is slightly ungrammatical. 'using' is sufficient and
more natural here.
```suggestion
"Please install it using "
```
##########
sdks/python/apache_beam/ml/transforms/embeddings/tensorflow_hub.py:
##########
@@ -41,6 +50,13 @@ class _TensorflowHubModelHandler(TFModelHandlerTensor):
Note: Intended for internal use only. No backwards compatibility guarantees.
"""
def __init__(self, preprocessing_url: Optional[str], *args, **kwargs):
+ if not (tf and hub and text):
+ raise ImportError(
+ "tensorflow, tensorflow_hub and tensorflow_text are required to use "
+ "TensorflowHubTextEmbeddings and TensorflowHubImageEmbeddings. "
+ "Please install them with using "
Review Comment:
The phrase 'with using' is slightly ungrammatical. 'using' is sufficient and
more natural here.
```suggestion
"Please install them using "
```
##########
sdks/python/apache_beam/yaml/yaml_ml.py:
##########
@@ -20,10 +20,14 @@
from typing import Any
from typing import Optional
+import pkgutil
+import importlib
Review Comment:
To support logging import errors when dynamically loading modules, please
add the `logging` import. This will help in debugging cases where a transform
is not available as expected.
```suggestion
import importlib
import logging
```
--
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]
