gemini-code-assist[bot] commented on code in PR #36093: URL: https://github.com/apache/beam/pull/36093#discussion_r2337469851
########## sdks/python/apache_beam/internal/pickler.py: ########## @@ -74,6 +78,12 @@ def load_session(file_path): def set_library(selected_library=DEFAULT_PICKLE_LIB): """ Sets pickle library that will be used. """ global desired_pickle_lib + + if selected_library == USE_DILL and not dill_pickler: + if not dill_pickler: + raise ImportError( + "Dill is not installed. To use dill pickler, install apache-beam with" + " the dill extras package e.g. apache-beam[dill].") Review Comment:  The nested `if not dill_pickler:` check is redundant because the outer `if` condition already verifies that `dill_pickler` is falsy. You can remove the inner `if` block to simplify the code. ```python if selected_library == USE_DILL and not dill_pickler: raise ImportError( "Dill is not installed. To use dill pickler, install apache-beam with" " the dill extras package e.g. apache-beam[dill].") ``` ########## sdks/python/apache_beam/coders/coders.py: ########## @@ -900,6 +898,12 @@ def to_type_hint(self): class DillCoder(_PickleCoderBase): """Coder using dill's pickle functionality.""" + def __init__(self): + if not dill: + raise RuntimeError("This pipeline contains a DillCoder which requires " \ + "the dill package. Install the dill package with the dill extra " \ + "e.g. apache-beam[dill]") Review Comment:  For multi-line strings, it's more idiomatic in Python to use implicit string concatenation within parentheses rather than using backslashes for line continuation. This improves readability and is less prone to formatting errors. ```suggestion raise RuntimeError( "This pipeline contains a DillCoder which requires the dill package. " "Install the dill package with the dill extra e.g. apache-beam[dill]") ``` ########## sdks/python/apache_beam/options/pipeline_options_validator.py: ########## @@ -196,6 +205,25 @@ def validate_gcs_path(self, view, arg_name): return self._validate_error(self.ERR_INVALID_GCS_OBJECT, arg, arg_name) return [] + def validate_pickle_library(self, view): + """Validates a GCS path against gs://bucket/object URI format.""" Review Comment:  The docstring for this function appears to be a copy-paste error and is misleading. It should describe that the function validates the `pickle_library` option, not a GCS path. ```suggestion """Validates the pickle_library option.""" ``` -- 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-unsubscr...@beam.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
