From 4667a7e7ad18adf42fa17a836ef350e20349baf1 Mon Sep 17 00:00:00 2001 From: smokestacklightnin <125844868+smokestacklightnin@users.noreply.github.com> Date: Sat, 26 Oct 2024 16:41:11 -0700 Subject: [PATCH 01/16] Automatic fixes for ruff `B` rules --- tfx/dsl/component/experimental/decorators.py | 2 +- tfx/dsl/component/experimental/utils.py | 12 ++---------- tfx/types/artifact_utils.py | 4 ++-- tfx/utils/deprecation_utils.py | 2 +- tfx/utils/deprecation_utils_test.py | 4 ++-- tfx/utils/import_utils_test.py | 4 +--- 6 files changed, 9 insertions(+), 19 deletions(-) diff --git a/tfx/dsl/component/experimental/decorators.py b/tfx/dsl/component/experimental/decorators.py index d83bd3cc18..4efe46e854 100644 --- a/tfx/dsl/component/experimental/decorators.py +++ b/tfx/dsl/component/experimental/decorators.py @@ -202,7 +202,7 @@ def __init__(self, *unused_args, **kwargs): json_compat_typehint = getattr(channel_parameter, '_JSON_COMPAT_TYPEHINT', None) if json_compat_typehint: - setattr(spec_kwargs[key], '_JSON_COMPAT_TYPEHINT', json_compat_typehint) + spec_kwargs[key]._JSON_COMPAT_TYPEHINT = json_compat_typehint spec = self.SPEC_CLASS(**spec_kwargs) super().__init__(spec) # Set class name, which is the decorated function name, as the default id. diff --git a/tfx/dsl/component/experimental/utils.py b/tfx/dsl/component/experimental/utils.py index 4d88692622..81e709f029 100644 --- a/tfx/dsl/component/experimental/utils.py +++ b/tfx/dsl/component/experimental/utils.py @@ -209,21 +209,13 @@ def _create_component_spec_class( type=artifact_type, optional=(key in arg_defaults) ) if json_compatible_inputs and key in json_compatible_inputs: - setattr( - spec_inputs[key], - '_JSON_COMPAT_TYPEHINT', - json_compatible_inputs[key], - ) + spec_inputs[key]._JSON_COMPAT_TYPEHINT = json_compatible_inputs[key] if outputs: for key, artifact_type in outputs.items(): assert key not in arg_defaults, 'Optional outputs are not supported.' spec_outputs[key] = component_spec.ChannelParameter(type=artifact_type) if json_compatible_outputs and key in json_compatible_outputs: - setattr( - spec_outputs[key], - '_JSON_COMPAT_TYPEHINT', - json_compatible_outputs[key], - ) + spec_outputs[key]._JSON_COMPAT_TYPEHINT = json_compatible_outputs[key] if parameters: for key, param_type in parameters.items(): if inspect.isclass(param_type) and issubclass( diff --git a/tfx/types/artifact_utils.py b/tfx/types/artifact_utils.py index 358400cbc4..04b0aa6486 100644 --- a/tfx/types/artifact_utils.py +++ b/tfx/types/artifact_utils.py @@ -188,11 +188,11 @@ def get_artifact_type_class( if artifact_type.name.startswith(cls.TYPE_NAME): new_artifact_class = _ValueArtifactType( mlmd_artifact_type=artifact_type, base=cls) - setattr(new_artifact_class, '_AUTOGENERATED', True) + new_artifact_class._AUTOGENERATED = True return new_artifact_class new_artifact_class = _ArtifactType(mlmd_artifact_type=artifact_type) - setattr(new_artifact_class, '_AUTOGENERATED', True) + new_artifact_class._AUTOGENERATED = True return new_artifact_class diff --git a/tfx/utils/deprecation_utils.py b/tfx/utils/deprecation_utils.py index 039da6504f..0a98541438 100644 --- a/tfx/utils/deprecation_utils.py +++ b/tfx/utils/deprecation_utils.py @@ -36,7 +36,7 @@ def _should_warn(func_or_class, warn_once=True): def _validate_callable(func): - if not hasattr(func, '__call__'): + if not callable(func): raise ValueError('%s passed to is not a function for deprecation.' % (func,)) diff --git a/tfx/utils/deprecation_utils_test.py b/tfx/utils/deprecation_utils_test.py index 43c218e132..d533630dce 100644 --- a/tfx/utils/deprecation_utils_test.py +++ b/tfx/utils/deprecation_utils_test.py @@ -36,8 +36,8 @@ def _mock_function(self, name='function'): """Return a mock function.""" function = mock.MagicMock() # Either `__qualname__` or `__name__` is expected to be set for a function. - setattr(function, '__qualname__', name) - setattr(function, '__name__', name) + function.__qualname__ = name + function.__name__ = name return function def testDeprecated(self): diff --git a/tfx/utils/import_utils_test.py b/tfx/utils/import_utils_test.py index 88050e9191..89d6fd7708 100644 --- a/tfx/utils/import_utils_test.py +++ b/tfx/utils/import_utils_test.py @@ -82,7 +82,5 @@ def testtestImportFuncFromModuleReload(self): """) fn_2 = import_utils.import_func_from_source(test_fn_file, 'test_fn') self.assertEqual(11, fn_2([1, 2, 3, 4])) - fn_3 = getattr( - importlib.reload(sys.modules['user_module_%d' % count_registered]), - 'test_fn') + fn_3 = importlib.reload(sys.modules['user_module_%d' % count_registered]).test_fn self.assertEqual(11, fn_3([1, 2, 3, 4])) From bd7ae6e4fccbe0a370e3913665f4b24c59c25c19 Mon Sep 17 00:00:00 2001 From: smokestacklightnin <125844868+smokestacklightnin@users.noreply.github.com> Date: Sat, 26 Oct 2024 17:25:54 -0700 Subject: [PATCH 02/16] Fix Ruff rule B904 --- tfx/components/example_gen/utils.py | 28 +++++++++++-------- tfx/components/trainer/rewriting/rewriter.py | 6 ++-- tfx/dsl/compiler/placeholder_utils.py | 16 +++++------ .../sklearn_predict_extractor_test.py | 4 +-- .../prediction_clients.py | 5 ++-- .../experimental/core/pipeline_ops.py | 22 +++++++-------- tfx/orchestration/kubeflow/test_utils.py | 2 +- .../launcher/kubernetes_component_launcher.py | 4 +-- tfx/orchestration/metadata.py | 4 +-- .../portable/kubernetes_executor_operator.py | 6 ++-- tfx/types/node_common.py | 4 +-- tfx/utils/import_utils.py | 4 +-- tfx/utils/kube_utils.py | 10 +++---- 13 files changed, 60 insertions(+), 55 deletions(-) diff --git a/tfx/components/example_gen/utils.py b/tfx/components/example_gen/utils.py index adc1313b5f..d1a26c2ac9 100644 --- a/tfx/components/example_gen/utils.py +++ b/tfx/components/example_gen/utils.py @@ -271,8 +271,8 @@ def _make_zero_padding_spec_value(spec_full_regex: str, pattern: str, width_int = 0 try: width_int = int(width_str) - except ValueError: - raise ValueError('Width modifier is not a integer: %s' % pattern) + except ValueError as e: + raise ValueError('Width modifier is not a integer: %s' % pattern) from e if width_int <= 0: raise ValueError('Width modifier is not positive: %s' % pattern) if width_int < len(str(spec_value)): @@ -377,30 +377,34 @@ def _find_matched_span_version_from_path( matched_span_tokens = [result.group(SPAN_PROPERTY_NAME)] try: matched_span_int = int(matched_span_tokens[0]) - except ValueError: + except ValueError as e: raise ValueError('Cannot find %s number from %s based on %s' % - (SPAN_PROPERTY_NAME, file_path, split_regex_pattern)) + (SPAN_PROPERTY_NAME, file_path, split_regex_pattern) + ) from e elif is_match_date: matched_span_tokens = [ result.group(name) for name in ['year', 'month', 'day'] ] try: matched_span_ints = [int(elem) for elem in matched_span_tokens] - except ValueError: + except ValueError as e: raise ValueError('Cannot find %s number using date from %s based on %s' % - (SPAN_PROPERTY_NAME, file_path, split_regex_pattern)) + (SPAN_PROPERTY_NAME, file_path, split_regex_pattern) + ) from e try: matched_span_int = date_to_span_number(*matched_span_ints) - except ValueError: - raise ValueError('Retrieved date is invalid for file: %s' % file_path) + except ValueError as e: + raise ValueError('Retrieved date is invalid for file: %s' % file_path + ) from e if is_match_version: matched_version = result.group(VERSION_PROPERTY_NAME) try: matched_version_int = int(matched_version) - except ValueError: + except ValueError as e: raise ValueError('Cannot find %s number from %s based on %s' % - (VERSION_PROPERTY_NAME, file_path, split_regex_pattern)) + (VERSION_PROPERTY_NAME, file_path, split_regex_pattern) + ) from e return (matched_span_tokens, matched_span_int, matched_version, matched_version_int) @@ -417,10 +421,10 @@ def _get_spec_width(spec_full_regex: str, spec_name: str, width_int = int(width_str) if width_int <= 0: raise ValueError('Not a positive integer.') - except ValueError: + except ValueError as e: raise ValueError( 'Width modifier in %s spec is not a positive integer: %s' % - (spec_name, split.pattern)) + (spec_name, split.pattern)) from e return width_str diff --git a/tfx/components/trainer/rewriting/rewriter.py b/tfx/components/trainer/rewriting/rewriter.py index dd69daf144..0aebe87383 100644 --- a/tfx/components/trainer/rewriting/rewriter.py +++ b/tfx/components/trainer/rewriting/rewriter.py @@ -100,18 +100,18 @@ def perform_rewrite(self, original_model: ModelDescription, raise ValueError('{} failed to perform pre-rewrite validation. Original ' 'model: {}. Error: {}'.format(self.name, str(original_model), - str(v))) + str(v))) from v try: self._rewrite(original_model, rewritten_model) except ValueError as v: raise ValueError( '{} failed to rewrite model. Original model: {}. Error {}'.format( - self.name, str(original_model), str(v))) + self.name, str(original_model), str(v))) from v try: self._post_rewrite_validate(rewritten_model) except ValueError as v: raise ValueError( '{} failed to validate rewritten model. Rewritten model: {}. Error {}' - .format(self.name, str(rewritten_model), str(v))) + .format(self.name, str(rewritten_model), str(v))) from v diff --git a/tfx/dsl/compiler/placeholder_utils.py b/tfx/dsl/compiler/placeholder_utils.py index a9a214dded..7b60cf0d57 100644 --- a/tfx/dsl/compiler/placeholder_utils.py +++ b/tfx/dsl/compiler/placeholder_utils.py @@ -391,9 +391,9 @@ def _resolve_property_operator( if op.is_custom_property: return value.get_custom_property(op.key) return value.__getattr__(op.key) - except: + except Exception as e: raise ValueError("ArtifactPropertyOperator failed to find property with " - f"key {op.key}.") + f"key {op.key}.") from e @_register(placeholder_pb2.Base64EncodeOperator) def _resolve_base64_encode_operator( @@ -514,18 +514,18 @@ def _resolve_proto_operator( if field.startswith("."): try: value = getattr(value, field[1:]) - except AttributeError: + except AttributeError as e: raise ValueError("While evaluting placeholder proto operator, " f"got unknown proto field {field} on proto of " - f"type {type(value)}.") + f"type {type(value)}.") from e continue map_key = re.findall(r"\[['\"](.+)['\"]\]", field) if len(map_key) == 1: try: value = value[map_key[0]] - except KeyError: + except KeyError as e: raise ValueError("While evaluting placeholder proto operator, " - f"got unknown map field {field}.") + f"got unknown map field {field}.") from e continue # Going forward, index access for proto fields should be handled by # index op. This code here is kept to avoid breaking existing executor @@ -534,9 +534,9 @@ def _resolve_proto_operator( if index and str.isdecimal(index[0]): try: value = value[int(index[0])] - except IndexError: + except IndexError as e: raise ValueError("While evaluting placeholder proto operator, " - f"got unknown index field {field}.") + f"got unknown index field {field}.") from e continue raise ValueError(f"Got unsupported proto field path: {field}") diff --git a/tfx/examples/penguin/experimental/sklearn_predict_extractor_test.py b/tfx/examples/penguin/experimental/sklearn_predict_extractor_test.py index 4b3e80c605..9f39ff1f24 100644 --- a/tfx/examples/penguin/experimental/sklearn_predict_extractor_test.py +++ b/tfx/examples/penguin/experimental/sklearn_predict_extractor_test.py @@ -94,7 +94,7 @@ def check_result(actual): self.assertEqual(item['labels'].shape, item['predictions'].shape) except AssertionError as err: - raise util.BeamAssertException(err) + raise util.BeamAssertException(err) from err util.assert_that(predict_extracts, check_result) @@ -145,7 +145,7 @@ def check_result(actual): self.assertIn('model2', item['predictions'][0]) except AssertionError as err: - raise util.BeamAssertException(err) + raise util.BeamAssertException(err) from err util.assert_that(predict_extracts, check_result) diff --git a/tfx/extensions/google_cloud_ai_platform/prediction_clients.py b/tfx/extensions/google_cloud_ai_platform/prediction_clients.py index a1c77cab41..3bff44f477 100644 --- a/tfx/extensions/google_cloud_ai_platform/prediction_clients.py +++ b/tfx/extensions/google_cloud_ai_platform/prediction_clients.py @@ -231,7 +231,7 @@ def deploy_model(self, logging.warn('Model version %s already exists', model_version_name) else: raise RuntimeError('Creating model version to AI Platform failed: {}' - .format(e)) + .format(e)) from e if set_default: # Set the new version as default. @@ -279,7 +279,8 @@ def create_model_for_aip_prediction_if_not_exist( logging.warn('Model %s already exists', model_name) result = False else: - raise RuntimeError('Creating model to AI Platform failed: {}'.format(e)) + raise RuntimeError('Creating model to AI Platform failed: {}'.format(e) + ) from e return result def _wait_for_operation(self, operation: Dict[str, Any], diff --git a/tfx/orchestration/experimental/core/pipeline_ops.py b/tfx/orchestration/experimental/core/pipeline_ops.py index 6774e23626..37cb48b1d7 100644 --- a/tfx/orchestration/experimental/core/pipeline_ops.py +++ b/tfx/orchestration/experimental/core/pipeline_ops.py @@ -187,7 +187,7 @@ def initiate_pipeline_start( except ValueError as e: raise status_lib.StatusNotOkError( code=status_lib.Code.INVALID_ARGUMENT, message=str(e) - ) + ) from e else: # Find all subpipelines in the parent pipeline, which we are caching. to_process = collections.deque([]) @@ -246,11 +246,11 @@ def initiate_pipeline_start( except ValueError as e: raise status_lib.StatusNotOkError( code=status_lib.Code.INVALID_ARGUMENT, message=str(e) - ) + ) from e except LookupError as e: raise status_lib.StatusNotOkError( code=status_lib.Code.FAILED_PRECONDITION, message=str(e) - ) + ) from e env.get_env().prepare_orchestrator_for_pipeline_run(pipeline) return pstate.PipelineState.new( mlmd_handle, pipeline, pipeline_run_metadata, reused_pipeline_view @@ -714,7 +714,7 @@ def delete_pipeline_run( except LookupError as e: raise status_lib.StatusNotOkError( code=status_lib.Code.NOT_FOUND, message=str(e) - ) + ) from e @_pipeline_op(lock=False) @@ -985,7 +985,7 @@ def resume_pipeline( except ValueError as e: raise status_lib.StatusNotOkError( code=status_lib.Code.INVALID_ARGUMENT, message=str(e) - ) + ) from e if pipeline.runtime_spec.HasField('snapshot_settings'): try: partial_run_utils.snapshot( @@ -994,11 +994,11 @@ def resume_pipeline( except ValueError as e: raise status_lib.StatusNotOkError( code=status_lib.Code.INVALID_ARGUMENT, message=str(e) - ) + ) from e except LookupError as e: raise status_lib.StatusNotOkError( code=status_lib.Code.FAILED_PRECONDITION, message=str(e) - ) + ) from e env.get_env().prepare_orchestrator_for_pipeline_run(pipeline) return pstate.PipelineState.new( mlmd_handle, pipeline, reused_pipeline_view=latest_pipeline_view @@ -1337,7 +1337,7 @@ def orchestrate( status_lib.StatusNotOkError: If error generating tasks. """ if filter_fn is None: - filter_fn = lambda _: True + filter_fn = lambda _: True # noqa: E731 # Try to load active pipelines. If there is a recoverable error, return True # and then retry in the next orchestration iteration. @@ -2208,7 +2208,7 @@ def _get_mlmd_protos_for_execution( ], ) except mlmd_errors.StatusError as e: - raise status_lib.StatusNotOkError(code=e.error_code, message=str(e)) + raise status_lib.StatusNotOkError(code=e.error_code, message=str(e)) from e output_artifact_ids = set() for event in lineage_graph.events: @@ -2332,7 +2332,7 @@ def publish_intermediate_artifact( except filesystem.NotFoundError as e: raise status_lib.StatusNotOkError( code=status_lib.Code.ABORTED, message=str(e) - ) + ) from e logging.info( 'Moved temporary URI %s contents to final URI %s', temp_uri, @@ -2391,7 +2391,7 @@ def publish_intermediate_artifact( ) except mlmd_errors.StatusError as e: - raise status_lib.StatusNotOkError(code=e.error_code, message=str(e)) + raise status_lib.StatusNotOkError(code=e.error_code, message=str(e)) from e logging.info('Published intermediate artifact: %s', intermediate_artifact) return intermediate_artifact diff --git a/tfx/orchestration/kubeflow/test_utils.py b/tfx/orchestration/kubeflow/test_utils.py index 50f87104ce..6a3ce2a100 100644 --- a/tfx/orchestration/kubeflow/test_utils.py +++ b/tfx/orchestration/kubeflow/test_utils.py @@ -112,7 +112,7 @@ def poll_kfp_with_retry(host: str, run_id: str, retry_limit: int, continue raise RuntimeError('Still hit remote error after %s retries: %s' % - (retry_limit, api_err)) + (retry_limit, api_err)) from api_err else: # If get_run succeeded, reset retry_count. retry_count = 0 diff --git a/tfx/orchestration/launcher/kubernetes_component_launcher.py b/tfx/orchestration/launcher/kubernetes_component_launcher.py index 353291e853..cc9c380d44 100644 --- a/tfx/orchestration/launcher/kubernetes_component_launcher.py +++ b/tfx/orchestration/launcher/kubernetes_component_launcher.py @@ -118,7 +118,7 @@ def _run_executor(self, execution_id: int, except client.rest.ApiException as e: raise RuntimeError( 'Failed to created container executor pod!\nReason: %s\nBody: %s' % - (e.reason, e.body)) + (e.reason, e.body)) from e # Wait up to 300 seconds for the pod to move from pending to another status. logging.info('Waiting for pod "%s:%s" to start.', namespace, pod_name) @@ -141,7 +141,7 @@ def _run_executor(self, execution_id: int, except client.rest.ApiException as e: raise RuntimeError( 'Failed to stream the logs from the pod!\nReason: %s\nBody: %s' % - (e.reason, e.body)) + (e.reason, e.body)) from e for log in logs: logging.info(log.decode().rstrip('\n')) diff --git a/tfx/orchestration/metadata.py b/tfx/orchestration/metadata.py index 427ec96fbd..67a035e6c6 100644 --- a/tfx/orchestration/metadata.py +++ b/tfx/orchestration/metadata.py @@ -433,14 +433,14 @@ def _prepare_execution_type(self, type_name: str, absl.logging.debug('Registering an execution type with id %s.' % execution_type_id) return execution_type_id - except mlmd.errors.AlreadyExistsError: + except mlmd.errors.AlreadyExistsError as e: # The conflict should not happen as all property value type is STRING. warning_str = ( 'Conflicting properties in exec_properties comparing with ' 'existing execution type with the same type name. Existing type: ' '%s, New type: %s') % (existing_execution_type, execution_type) absl.logging.warning(warning_str) - raise ValueError(warning_str) + raise ValueError(warning_str) from e def _update_execution_proto( self, diff --git a/tfx/orchestration/portable/kubernetes_executor_operator.py b/tfx/orchestration/portable/kubernetes_executor_operator.py index 86ece8346b..d707b3723d 100644 --- a/tfx/orchestration/portable/kubernetes_executor_operator.py +++ b/tfx/orchestration/portable/kubernetes_executor_operator.py @@ -14,7 +14,7 @@ """Docker component launcher which launches a container in docker environment .""" import collections -from typing import Any, Dict, List, Optional, cast +from typing import Any, Dict, Optional, cast from absl import logging from kubernetes import client @@ -117,7 +117,7 @@ def run_executor( except client.rest.ApiException as e: raise RuntimeError( 'Failed to created container executor pod!\nReason: %s\nBody: %s' % - (e.reason, e.body)) + (e.reason, e.body)) from e # Wait up to 300 seconds for the pod to move from pending to another status. logging.info('Waiting for pod "%s:%s" to start.', namespace, pod_name) @@ -140,7 +140,7 @@ def run_executor( except client.rest.ApiException as e: raise RuntimeError( 'Failed to stream the logs from the pod!\nReason: %s\nBody: %s' % - (e.reason, e.body)) + (e.reason, e.body)) from e for log in logs: logging.info(log.decode().rstrip('\n')) diff --git a/tfx/types/node_common.py b/tfx/types/node_common.py index e18fd1d690..d4de3f7ef1 100644 --- a/tfx/types/node_common.py +++ b/tfx/types/node_common.py @@ -47,8 +47,8 @@ def __getattr__(self, key): key = self._compat_aliases[key] try: return self._data[key] - except KeyError: - raise AttributeError + except KeyError as e: + raise AttributeError from e def __repr__(self): return repr(self._data) diff --git a/tfx/utils/import_utils.py b/tfx/utils/import_utils.py index ae0ee4de14..19e9582ad8 100644 --- a/tfx/utils/import_utils.py +++ b/tfx/utils/import_utils.py @@ -152,10 +152,10 @@ def import_func_from_source(source_path: str, fn_name: str) -> Callable: # pyli sys.modules[loader.name] = module loader.exec_module(module) _tfx_module_finder.register_module(module_name, source_path) - except IOError: + except IOError as e: raise ImportError('{} in {} not found in ' 'import_func_from_source()'.format( - fn_name, source_path)) + fn_name, source_path)) from e else: logging.info('%s is already loaded, reloading', source_path) module_name = _tfx_module_finder.get_module_name_by_path(source_path) diff --git a/tfx/utils/kube_utils.py b/tfx/utils/kube_utils.py index 5aac3a9f52..df06dcf6e9 100644 --- a/tfx/utils/kube_utils.py +++ b/tfx/utils/kube_utils.py @@ -254,9 +254,9 @@ def get_kfp_namespace() -> str: """ try: return os.environ[KFP_NAMESPACE] - except KeyError: + except KeyError as e: raise RuntimeError( - 'Cannot determine KFP namespace from the environment.') + 'Cannot determine KFP namespace from the environment.') from e def get_current_kfp_pod(client: k8s_client.CoreV1Api) -> k8s_client.V1Pod: @@ -274,8 +274,8 @@ def get_current_kfp_pod(client: k8s_client.CoreV1Api) -> k8s_client.V1Pod: namespace = os.environ[KFP_NAMESPACE] pod_name = os.environ[KFP_POD_NAME] return client.read_namespaced_pod(name=pod_name, namespace=namespace) - except KeyError: - raise RuntimeError('Cannot determine KFP pod from the environment.') + except KeyError as e: + raise RuntimeError('Cannot determine KFP pod from the environment.') from e def get_pod(core_api: k8s_client.CoreV1Api, pod_name: str, @@ -297,7 +297,7 @@ def get_pod(core_api: k8s_client.CoreV1Api, pod_name: str, except k8s_client.rest.ApiException as e: if e.status != 404: raise RuntimeError('Unknown error! \nReason: %s\nBody: %s' % - (e.reason, e.body)) + (e.reason, e.body)) from e return None From 98bfa94566e01b28b1f3197e21366f2a35c5764a Mon Sep 17 00:00:00 2001 From: smokestacklightnin <125844868+smokestacklightnin@users.noreply.github.com> Date: Sat, 26 Oct 2024 17:31:24 -0700 Subject: [PATCH 03/16] Fix Ruff rule B026 --- tfx/utils/json_utils.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tfx/utils/json_utils.py b/tfx/utils/json_utils.py index d674c9dab1..c96ddc866f 100644 --- a/tfx/utils/json_utils.py +++ b/tfx/utils/json_utils.py @@ -92,14 +92,14 @@ class _DefaultEncoder(json.JSONEncoder): def encode(self, obj: Any) -> str: """Override encode to prevent redundant dumping.""" - if obj.__class__.__name__ == 'RuntimeParameter' and obj.ptype == str: + if obj.__class__.__name__ == 'RuntimeParameter' and obj.ptype is str: return self.default(obj) return super().encode(obj) def default(self, obj: Any) -> Any: # If obj is a str-typed RuntimeParameter, serialize it in place. - if obj.__class__.__name__ == 'RuntimeParameter' and obj.ptype == str: + if obj.__class__.__name__ == 'RuntimeParameter' and obj.ptype is str: dict_data = { _TFX_OBJECT_TYPE_KEY: _ObjectType.JSONABLE, _MODULE_KEY: obj.__class__.__module__, @@ -117,7 +117,7 @@ def default(self, obj: Any) -> Any: # Need to first check the existence of str-typed runtime parameter. data_patch = obj.to_json_dict() for k, v in data_patch.items(): - if v.__class__.__name__ == 'RuntimeParameter' and v.ptype == str: + if v.__class__.__name__ == 'RuntimeParameter' and v.ptype is str: data_patch[k] = dumps(v) dict_data.update(data_patch) return dict_data @@ -148,7 +148,7 @@ class _DefaultDecoder(json.JSONDecoder): def __init__(self, *args, **kwargs): super().__init__( - object_hook=self._dict_to_object, *args, **kwargs) + object_hook=self._dict_to_object, *args, **kwargs) # noqa: B026 def _dict_to_object(self, dict_data: Dict[str, Any]) -> Any: """Converts a dictionary to an object.""" From 500ee55f85a133f3549a696892c2b0a2312a6c20 Mon Sep 17 00:00:00 2001 From: smokestacklightnin <125844868+smokestacklightnin@users.noreply.github.com> Date: Sat, 26 Oct 2024 17:35:51 -0700 Subject: [PATCH 04/16] Fix Ruff rule B028 --- tfx/orchestration/airflow/airflow_dag_runner.py | 6 ++++-- tfx/orchestration/pipeline.py | 6 ++++-- tfx/utils/deprecation_utils.py | 2 +- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/tfx/orchestration/airflow/airflow_dag_runner.py b/tfx/orchestration/airflow/airflow_dag_runner.py index a6a63c3390..61ff7f320e 100644 --- a/tfx/orchestration/airflow/airflow_dag_runner.py +++ b/tfx/orchestration/airflow/airflow_dag_runner.py @@ -64,7 +64,9 @@ def __init__(self, if config and not isinstance(config, AirflowPipelineConfig): warnings.warn( 'Pass config as a dict type is going to deprecated in 0.1.16. ' - 'Use AirflowPipelineConfig type instead.', PendingDeprecationWarning) + 'Use AirflowPipelineConfig type instead.', + PendingDeprecationWarning, + stacklevel=2) config = AirflowPipelineConfig(airflow_dag_config=config) super().__init__(config) @@ -123,7 +125,7 @@ def _replace_runtime_params(self, comp): for k, prop in comp.exec_properties.copy().items(): if isinstance(prop, RuntimeParameter): # Airflow only supports string parameters. - if prop.ptype != str: + if prop.ptype is not str: raise RuntimeError( f'RuntimeParameter in Airflow does not support {prop.ptype}. The' 'only ptype supported is string.') diff --git a/tfx/orchestration/pipeline.py b/tfx/orchestration/pipeline.py index cd7e88cea7..54a2c8f70b 100644 --- a/tfx/orchestration/pipeline.py +++ b/tfx/orchestration/pipeline.py @@ -414,7 +414,8 @@ def _persist_dsl_context_registry(self): 'This is probably due to reusing component from another pipeline ' 'or interleaved pipeline definitions. Make sure each component ' 'belong to exactly one pipeline, and pipeline definitions are ' - 'separated.') + 'separated.', + stacklevel=2) @property def inputs(self) -> Dict[str, Any]: @@ -507,5 +508,6 @@ def enumerate_implicit_dependencies( warnings.warn( f'Node {component.id} depends on the output of node' f' {upstream_node_id}, but {upstream_node_id} is not included in' - ' the components of pipeline. Did you forget to add it?' + ' the components of pipeline. Did you forget to add it?', + stacklevel=2 ) diff --git a/tfx/utils/deprecation_utils.py b/tfx/utils/deprecation_utils.py index 0a98541438..3e4cbc7566 100644 --- a/tfx/utils/deprecation_utils.py +++ b/tfx/utils/deprecation_utils.py @@ -185,4 +185,4 @@ class TfxDeprecationWarning(DeprecationWarning): # pylint: disable=g-bad-except def warn_deprecated(msg): """Convenient method to warn TfxDeprecationWarning.""" - warnings.warn(msg, TfxDeprecationWarning) + warnings.warn(msg, TfxDeprecationWarning, stacklevel=2) From 550298bb6a4907c8b42ed4571b3abb66950520de Mon Sep 17 00:00:00 2001 From: smokestacklightnin <125844868+smokestacklightnin@users.noreply.github.com> Date: Sat, 26 Oct 2024 17:37:46 -0700 Subject: [PATCH 05/16] Fix Ruff rule B018 --- tfx/orchestration/mlmd_connection_manager_test.py | 6 +++--- tfx/types/artifact_test.py | 6 +++--- tfx/types/value_artifact_test.py | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/tfx/orchestration/mlmd_connection_manager_test.py b/tfx/orchestration/mlmd_connection_manager_test.py index f0f01fde09..a25dc90821 100644 --- a/tfx/orchestration/mlmd_connection_manager_test.py +++ b/tfx/orchestration/mlmd_connection_manager_test.py @@ -45,7 +45,7 @@ def test_primary_handle(self): def test_unusable_without_enter(self): cm = mlmd_cm.MLMDConnectionManager.fake() with self.assertRaisesRegex(RuntimeError, 'not entered yet'): - cm.primary_mlmd_handle # pylint: disable=pointless-statement + cm.primary_mlmd_handle # noqa: B018 def test_enter_synced_with_handle(self): cm = mlmd_cm.MLMDConnectionManager.fake() @@ -54,7 +54,7 @@ def test_enter_synced_with_handle(self): self.assertIsNotNone(handle.store) with self.assertRaisesRegex( RuntimeError, 'Metadata object is not in enter state'): - handle.store # pylint: disable=pointless-statement + handle.store # noqa: B018 def test_multiple_enterable(self): cm = mlmd_cm.MLMDConnectionManager.fake() @@ -64,4 +64,4 @@ def test_multiple_enterable(self): m2 = cm.primary_mlmd_handle self.assertIs(m1, m2) with self.assertRaises(RuntimeError): - cm.primary_mlmd_handle # pylint: disable=pointless-statement + cm.primary_mlmd_handle # noqa: B018 diff --git a/tfx/types/artifact_test.py b/tfx/types/artifact_test.py index 006ccf030e..b9d82430ed 100644 --- a/tfx/types/artifact_test.py +++ b/tfx/types/artifact_test.py @@ -175,10 +175,10 @@ def testArtifact(self): # Default property does not have span or split_names. with self.assertRaisesRegex(AttributeError, "has no property 'span'"): - instance.span # pylint: disable=pointless-statement + instance.span # noqa: B018 with self.assertRaisesRegex(AttributeError, "has no property 'split_names'"): - instance.split_names # pylint: disable=pointless-statement + instance.split_names # noqa: B018 # Test property setters. instance.name = 'test_artifact' @@ -1229,7 +1229,7 @@ def testArtifactProperties(self): with self.assertRaisesRegex(AttributeError, "Artifact has no property 'invalid'"): - my_artifact.invalid # pylint: disable=pointless-statement + my_artifact.invalid # noqa: B018 def testStringTypeNameNotAllowed(self): with self.assertRaisesRegex( diff --git a/tfx/types/value_artifact_test.py b/tfx/types/value_artifact_test.py index 0a542652f4..bf6cbd1811 100644 --- a/tfx/types/value_artifact_test.py +++ b/tfx/types/value_artifact_test.py @@ -109,7 +109,7 @@ def testValueArtifact(self): with self.assertRaisesRegex( ValueError, 'The artifact value has not yet been read from storage.'): - instance.value # pylint: disable=pointless-statement + instance.value # noqa: B018 instance.read() instance.value = _STRING_VALUE From 18a4d3cb5710491d86b7ecb88080841fea8aa37c Mon Sep 17 00:00:00 2001 From: smokestacklightnin <125844868+smokestacklightnin@users.noreply.github.com> Date: Sat, 26 Oct 2024 17:39:09 -0700 Subject: [PATCH 06/16] Fix Ruff rule B007 --- tfx/orchestration/portable/outputs_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tfx/orchestration/portable/outputs_utils.py b/tfx/orchestration/portable/outputs_utils.py index 971a593b3f..75169f8d2e 100644 --- a/tfx/orchestration/portable/outputs_utils.py +++ b/tfx/orchestration/portable/outputs_utils.py @@ -346,7 +346,7 @@ def tag_output_artifacts_with_version( """Tag output artifacts with the current TFX version.""" if not output_artifacts: return - for unused_key, artifact_list in output_artifacts.items(): + for _unused_key, artifact_list in output_artifacts.items(): for artifact in artifact_list: if not artifact.has_custom_property( artifact_utils.ARTIFACT_TFX_VERSION_CUSTOM_PROPERTY_KEY): From 9724828e046e41bce69b012928307df0f2f1d3e2 Mon Sep 17 00:00:00 2001 From: smokestacklightnin <125844868+smokestacklightnin@users.noreply.github.com> Date: Sat, 26 Oct 2024 17:49:17 -0700 Subject: [PATCH 07/16] Fix Ruff rule B017 --- .../google_cloud_ai_platform/bulk_inferrer/executor_test.py | 2 +- tfx/orchestration/experimental/core/pipeline_ops_test.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tfx/extensions/google_cloud_ai_platform/bulk_inferrer/executor_test.py b/tfx/extensions/google_cloud_ai_platform/bulk_inferrer/executor_test.py index b8f25f2d36..0181f59090 100644 --- a/tfx/extensions/google_cloud_ai_platform/bulk_inferrer/executor_test.py +++ b/tfx/extensions/google_cloud_ai_platform/bulk_inferrer/executor_test.py @@ -235,7 +235,7 @@ def testDoFailedModelDeployment(self, mock_runner, mock_run_model_inference, mock_runner.create_model_for_aip_prediction_if_not_exist.return_value = True bulk_inferrer = executor.Executor(self._context) - with self.assertRaises(Exception): + with self.assertRaises(Exception): # noqa: B017 bulk_inferrer.Do(input_dict, output_dict, exec_properties) mock_runner.delete_model_from_aip_if_exists.assert_called_once_with( diff --git a/tfx/orchestration/experimental/core/pipeline_ops_test.py b/tfx/orchestration/experimental/core/pipeline_ops_test.py index 17cc405865..98e28934ea 100644 --- a/tfx/orchestration/experimental/core/pipeline_ops_test.py +++ b/tfx/orchestration/experimental/core/pipeline_ops_test.py @@ -3803,7 +3803,7 @@ def test_orchestrate_pipelines_with_not_recoverable_error_from_MLMD( with self._mlmd_cm as mlmd_connection_manager: task_queue = tq.TaskQueue() - with self.assertRaises(Exception): + with self.assertRaises(Exception): # noqa: B017 pipeline_ops.orchestrate( mlmd_connection_manager, task_queue, From ad106168b59698cb41a40e720160093cb686b30f Mon Sep 17 00:00:00 2001 From: smokestacklightnin <125844868+smokestacklightnin@users.noreply.github.com> Date: Sat, 26 Oct 2024 17:59:03 -0700 Subject: [PATCH 08/16] Fix some Ruff rule B008 violations. For remaining B008 violations, see [Issue 6945](https://github.com/tensorflow/tfx/issues/6945) --- tfx/examples/bert/utils/bert_models.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tfx/examples/bert/utils/bert_models.py b/tfx/examples/bert/utils/bert_models.py index d67fa1c6b0..cf59e9ac41 100644 --- a/tfx/examples/bert/utils/bert_models.py +++ b/tfx/examples/bert/utils/bert_models.py @@ -13,6 +13,7 @@ # limitations under the License. """Configurable fine-tuning BERT models for various tasks.""" +from __future__ import annotations from typing import Optional, List, Union import tensorflow as tf @@ -59,8 +60,7 @@ def build_bert_classifier(bert_layer: tf.keras.layers.Layer, def compile_bert_classifier( model: tf.keras.Model, - loss: tf.keras.losses.Loss = tf.keras.losses.SparseCategoricalCrossentropy( - from_logits=True), + loss: tf.keras.losses.Loss | None = None, learning_rate: float = 2e-5, metrics: Optional[List[Union[str, tf.keras.metrics.Metric]]] = None): """Compile the BERT classifier using suggested parameters. @@ -79,6 +79,9 @@ def compile_bert_classifier( Returns: None. """ + if loss is None: + loss = tf.keras.losses.SparseCategoricalCrossentropy(from_logits=True) + if metrics is None: metrics = ["sparse_categorical_accuracy"] From 0fbbfb5d84d837a6dc86d4c62046001d9d7797b7 Mon Sep 17 00:00:00 2001 From: smokestacklightnin <125844868+smokestacklightnin@users.noreply.github.com> Date: Sat, 26 Oct 2024 18:02:44 -0700 Subject: [PATCH 09/16] Fix Ruff rule B023 --- tfx/components/example_diff/executor.py | 6 +++++- tfx/dsl/placeholder/proto_placeholder.py | 4 ++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/tfx/components/example_diff/executor.py b/tfx/components/example_diff/executor.py index ca888035ec..e805cbb920 100644 --- a/tfx/components/example_diff/executor.py +++ b/tfx/components/example_diff/executor.py @@ -175,7 +175,11 @@ def Do(self, input_dict: Dict[str, List[types.Artifact]], logging.info('Processing split pair %s', split_pair) # pylint: disable=cell-var-from-loop @beam.ptransform_fn - def _iteration(p): + def _iteration(p, + test_tfxio=test_tfxio, + base_tfxio=base_tfxio, + split_pair=split_pair + ): base_examples = ( p | 'TFXIORead[base]' >> test_tfxio.RawRecordBeamSource() | 'Parse[base]' >> beam.Map(_parse_example)) diff --git a/tfx/dsl/placeholder/proto_placeholder.py b/tfx/dsl/placeholder/proto_placeholder.py index ebb79ca183..d677fffadf 100644 --- a/tfx/dsl/placeholder/proto_placeholder.py +++ b/tfx/dsl/placeholder/proto_placeholder.py @@ -509,11 +509,11 @@ def _purge_types( _purge_types(name_prefix, message_descriptor) # Step 2 _remove_unless( file_descriptor.message_type, - lambda m: f'{name_prefix}.{m.name}' in self._keep_types, # pylint: disable=cell-var-from-loop + lambda m, name_prefix=name_prefix: f'{name_prefix}.{m.name}' in self._keep_types, # pylint: disable=cell-var-from-loop ) _remove_unless( file_descriptor.enum_type, - lambda e: f'{name_prefix}.{e.name}' in self._keep_types, # pylint: disable=cell-var-from-loop + lambda e, name_prefix=name_prefix: f'{name_prefix}.{e.name}' in self._keep_types, # pylint: disable=cell-var-from-loop ) # Step 4: Remove file descriptors that became empty. Remove declared From 8bc3fd3786acd36b1b0f0e810552993c94053698 Mon Sep 17 00:00:00 2001 From: smokestacklightnin <125844868+smokestacklightnin@users.noreply.github.com> Date: Sat, 26 Oct 2024 19:09:18 -0700 Subject: [PATCH 10/16] Fix Ruff rule ISC linting violations --- tfx/examples/penguin/penguin_utils_cloud_tuner.py | 2 +- tfx/experimental/templates/container_based_test_case.py | 2 +- tfx/types/artifact.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tfx/examples/penguin/penguin_utils_cloud_tuner.py b/tfx/examples/penguin/penguin_utils_cloud_tuner.py index 15b02c83ed..38def78e58 100644 --- a/tfx/examples/penguin/penguin_utils_cloud_tuner.py +++ b/tfx/examples/penguin/penguin_utils_cloud_tuner.py @@ -229,7 +229,7 @@ def tuner_fn(fn_args: tfx.components.FnArgs) -> tfx.components.TunerFnResult: datetime.datetime.now().strftime('%Y%m%d%H')) if _CLOUD_FIT_IMAGE == 'gcr.io/my-project-id/cloud_fit': - raise ValueError('Build your own cloud_fit image, ' + + raise ValueError('Build your own cloud_fit image, ' 'default dummy one is used!') tuner = cloud_tuner.DistributingCloudTuner( diff --git a/tfx/experimental/templates/container_based_test_case.py b/tfx/experimental/templates/container_based_test_case.py index 6be904038b..d241d3e108 100644 --- a/tfx/experimental/templates/container_based_test_case.py +++ b/tfx/experimental/templates/container_based_test_case.py @@ -102,7 +102,7 @@ def _prepare_data(self): io_utils.copy_file( 'data/data.csv', f'gs://{self._BUCKET_NAME}/{self._DATA_DIRECTORY_NAME}/' - + f'{self._pipeline_name}/data.csv') + f'{self._pipeline_name}/data.csv') @retry.retry(ignore_eventual_failure=True) def _delete_pipeline_data(self): diff --git a/tfx/types/artifact.py b/tfx/types/artifact.py index df626d231f..40fedfdd7f 100644 --- a/tfx/types/artifact.py +++ b/tfx/types/artifact.py @@ -247,7 +247,7 @@ def _get_artifact_type(cls): if type_annotation_cls: if not issubclass(type_annotation_cls, SystemArtifact): raise ValueError( - '%s''s TYPE_ANNOTATION %s is not a subclass of SystemArtifact.' % + '%ss TYPE_ANNOTATION %s is not a subclass of SystemArtifact.' % (cls, type_annotation_cls)) if type_annotation_cls.MLMD_SYSTEM_BASE_TYPE: artifact_type.base_type = type_annotation_cls.MLMD_SYSTEM_BASE_TYPE From 6c36eaeb57211dc70f94c2d5de37026283318f60 Mon Sep 17 00:00:00 2001 From: smokestacklightnin <125844868+smokestacklightnin@users.noreply.github.com> Date: Sat, 26 Oct 2024 19:15:29 -0700 Subject: [PATCH 11/16] Fix Ruff rule A002 --- tfx/dsl/compiler/node_inputs_compiler_test.py | 2 +- tfx/dsl/components/base/base_component_test.py | 2 +- tfx/dsl/components/base/base_node.py | 4 ++-- tfx/dsl/input_resolution/canned_resolver_functions.py | 2 +- tfx/dsl/input_resolution/ops/test_utils.py | 4 ++-- tfx/dsl/input_resolution/resolver_op.py | 2 +- tfx/dsl/io/fileio.py | 6 +++--- tfx/dsl/placeholder/artifact_placeholder.py | 2 +- tfx/orchestration/pipeline_test.py | 4 ++-- tfx/types/artifact_property.py | 2 +- tfx/types/channel.py | 8 ++++---- tfx/types/component_spec.py | 4 ++-- tfx/v1/dsl/io/fileio.py | 2 +- tfx/v1/dsl/placeholders/__init__.py | 2 +- 14 files changed, 23 insertions(+), 23 deletions(-) diff --git a/tfx/dsl/compiler/node_inputs_compiler_test.py b/tfx/dsl/compiler/node_inputs_compiler_test.py index eadb97de48..edc529f969 100644 --- a/tfx/dsl/compiler/node_inputs_compiler_test.py +++ b/tfx/dsl/compiler/node_inputs_compiler_test.py @@ -47,7 +47,7 @@ class DummyArtifact(types.Artifact): class DummyNode(base_node.BaseNode): - def __init__(self, id: str, inputs=None, exec_properties=None): # pylint: disable=redefined-builtin + def __init__(self, id: str, inputs=None, exec_properties=None): # noqa: A002 super().__init__() self.with_id(id) self._inputs = inputs or {} diff --git a/tfx/dsl/components/base/base_component_test.py b/tfx/dsl/components/base/base_component_test.py index 93baa2d929..7a37123ec7 100644 --- a/tfx/dsl/components/base/base_component_test.py +++ b/tfx/dsl/components/base/base_component_test.py @@ -61,7 +61,7 @@ class _BasicComponent(base_component.BaseComponent): def __init__(self, spec: types.ComponentSpec = None, folds: int = None, - input: types.Channel = None): # pylint: disable=redefined-builtin + input: types.Channel = None): # noqa: A002 if not spec: output = types.Channel(type=_OutputArtifact) spec = _BasicComponentSpec(folds=folds, input=input, output=output) diff --git a/tfx/dsl/components/base/base_node.py b/tfx/dsl/components/base/base_node.py index a737f2da3d..c7e9efbefd 100644 --- a/tfx/dsl/components/base/base_node.py +++ b/tfx/dsl/components/base/base_node.py @@ -126,12 +126,12 @@ def component_id(self) -> str: @id.setter @doc_controls.do_not_doc_in_subclasses - def id(self, id: str) -> None: # pylint: disable=redefined-builtin + def id(self, id: str) -> None: # noqa: A002 self._id = id # TODO(kmonte): Update this to Self once we're on 3.11 everywhere @doc_controls.do_not_doc_in_subclasses - def with_id(self, id: str) -> typing_extensions.Self: # pylint: disable=redefined-builtin + def with_id(self, id: str) -> typing_extensions.Self: # noqa: A002 self._id = id return self diff --git a/tfx/dsl/input_resolution/canned_resolver_functions.py b/tfx/dsl/input_resolution/canned_resolver_functions.py index 734ca7a098..521fc55b42 100644 --- a/tfx/dsl/input_resolution/canned_resolver_functions.py +++ b/tfx/dsl/input_resolution/canned_resolver_functions.py @@ -744,7 +744,7 @@ def pick(channel: channel_types.BaseChannel, i: int, /): return _slice(channel, start=i, stop=(i + 1) or None, min_count=1) -def slice( # pylint: disable=redefined-builtin +def slice( # noqa: A002 channel: channel_types.BaseChannel, /, start: Optional[int] = None, diff --git a/tfx/dsl/input_resolution/ops/test_utils.py b/tfx/dsl/input_resolution/ops/test_utils.py index 1d4b0705b5..ba0ca08b61 100644 --- a/tfx/dsl/input_resolution/ops/test_utils.py +++ b/tfx/dsl/input_resolution/ops/test_utils.py @@ -51,7 +51,7 @@ class DummyArtifact(types.Artifact): 'version': tfx_artifact.Property(type=tfx_artifact.PropertyType.INT), } - # pylint: disable=redefined-builtin + # noqa: A002 def __init__( self, id: Optional[str] = None, @@ -149,7 +149,7 @@ class FakeComponent(base_component.BaseComponent): DRIVER_CLASS = base_driver.BaseDriver - def __init__(self, id: str, inputs=None, exec_properties=None): # pylint: disable=redefined-builtin + def __init__(self, id: str, inputs=None, exec_properties=None): # noqa: A002 super().__init__(spec=FakeSpec()) self.with_id(id) diff --git a/tfx/dsl/input_resolution/resolver_op.py b/tfx/dsl/input_resolution/resolver_op.py index b27f79649e..89e431b973 100644 --- a/tfx/dsl/input_resolution/resolver_op.py +++ b/tfx/dsl/input_resolution/resolver_op.py @@ -228,7 +228,7 @@ def my_resolver_stem(input_dict): def __init__( self, *, - type: Type[_T], # pylint: disable=redefined-builtin + type: Type[_T], # noqa: A002 default: Union[_T, _Empty] = _EMPTY): self._type = type self._required = default is _EMPTY diff --git a/tfx/dsl/io/fileio.py b/tfx/dsl/io/fileio.py index 5c540c2e5f..e700191f62 100644 --- a/tfx/dsl/io/fileio.py +++ b/tfx/dsl/io/fileio.py @@ -20,8 +20,8 @@ from tfx.dsl.io.filesystem import PathType # Import modules that may provide filesystem plugins. -import tfx.dsl.io.plugins.tensorflow_gfile # pylint: disable=unused-import, g-import-not-at-top -import tfx.dsl.io.plugins.local # pylint: disable=unused-import, g-import-not-at-top +import tfx.dsl.io.plugins.tensorflow_gfile # noqa: F401, E402 +import tfx.dsl.io.plugins.local # noqa: F401, E402 # Expose `NotFoundError` as `fileio.NotFoundError`. @@ -33,7 +33,7 @@ def _get_filesystem(path) -> Type[filesystem.Filesystem]: .get_filesystem_for_path(path)) -def open(path: PathType, mode: str = 'r'): # pylint: disable=redefined-builtin +def open(path: PathType, mode: str = 'r'): # noqa: A002 """Open a file at the given path.""" return _get_filesystem(path).open(path, mode=mode) diff --git a/tfx/dsl/placeholder/artifact_placeholder.py b/tfx/dsl/placeholder/artifact_placeholder.py index 9ab75d205e..161ccea0f9 100644 --- a/tfx/dsl/placeholder/artifact_placeholder.py +++ b/tfx/dsl/placeholder/artifact_placeholder.py @@ -23,7 +23,7 @@ _types = placeholder_base.types -def input(key: str) -> ArtifactPlaceholder: # pylint: disable=redefined-builtin +def input(key: str) -> ArtifactPlaceholder: # noqa: A002 """Returns a Placeholder that represents an input artifact. Args: diff --git a/tfx/orchestration/pipeline_test.py b/tfx/orchestration/pipeline_test.py index 85da251ea4..e3f4ee369c 100644 --- a/tfx/orchestration/pipeline_test.py +++ b/tfx/orchestration/pipeline_test.py @@ -85,7 +85,7 @@ class _FakeComponent(base_component.BaseComponent): def __init__( self, - type: Type[types.Artifact], # pylint: disable=redefined-builtin + type: Type[types.Artifact], # noqa: A002 spec_kwargs: Dict[str, Any]): spec = _FakeComponentSpec(output=types.Channel(type=type), **spec_kwargs) super().__init__(spec=spec) @@ -98,7 +98,7 @@ class _FakeBeamComponent(base_beam_component.BaseBeamComponent): def __init__( self, - type: Type[types.Artifact], # pylint: disable=redefined-builtin + type: Type[types.Artifact], # noqa: A002 spec_kwargs: Dict[str, Any]): spec = _FakeComponentSpec(output=types.Channel(type=type), **spec_kwargs) super().__init__(spec=spec) diff --git a/tfx/types/artifact_property.py b/tfx/types/artifact_property.py index f088da1288..8d64ba3c53 100644 --- a/tfx/types/artifact_property.py +++ b/tfx/types/artifact_property.py @@ -49,7 +49,7 @@ class Property: PropertyType.BOOLEAN: metadata_store_pb2.BOOLEAN, } - def __init__(self, type): # pylint: disable=redefined-builtin + def __init__(self, type): # noqa: A002 if type not in Property._ALLOWED_MLMD_TYPES: raise ValueError('Property type must be one of %s.' % list(Property._ALLOWED_MLMD_TYPES.keys())) diff --git a/tfx/types/channel.py b/tfx/types/channel.py index a00c4c3bbc..7d831d81d7 100644 --- a/tfx/types/channel.py +++ b/tfx/types/channel.py @@ -123,7 +123,7 @@ class BaseChannel(abc.ABC, Generic[_AT]): set. """ - def __init__(self, type: Type[_AT], is_optional: Optional[bool] = None): # pylint: disable=redefined-builtin + def __init__(self, type: Type[_AT], is_optional: Optional[bool] = None): # noqa: A002 if not _is_artifact_type(type): raise ValueError( 'Argument "type" of BaseChannel constructor must be a subclass of ' @@ -155,11 +155,11 @@ def as_optional(self) -> typing_extensions.Self: return new_channel @property - def type(self) -> Type[_AT]: # pylint: disable=redefined-builtin + def type(self) -> Type[_AT]: # noqa: A002 return self._artifact_type @type.setter - def type(self, value: Type[_AT]): # pylint: disable=redefined-builtin + def type(self, value: Type[_AT]): # noqa: A002 self._set_type(value) @doc_controls.do_not_generate_docs @@ -235,7 +235,7 @@ class won't be removed in TFX 1.x due to backward compatibility guarantee # TODO(b/125348988): Add support for real Channel in addition to static ones. def __init__( self, - type: Type[Artifact], # pylint: disable=redefined-builtin + type: Type[Artifact], # noqa: A002 additional_properties: Optional[Dict[str, Property]] = None, additional_custom_properties: Optional[Dict[str, Property]] = None, # TODO(b/161490287): deprecate static artifact. diff --git a/tfx/types/component_spec.py b/tfx/types/component_spec.py index d9e596d5c3..b5d29f99ff 100644 --- a/tfx/types/component_spec.py +++ b/tfx/types/component_spec.py @@ -340,7 +340,7 @@ class MyCustomComponentSpec(ComponentSpec): non-primitive types like lists) should be stored in its original form. """ - def __init__(self, type=None, optional=False, use_proto=False): # pylint: disable=redefined-builtin + def __init__(self, type=None, optional=False, use_proto=False): # noqa: A002 self.type = type self.optional = optional self.use_proto = use_proto @@ -419,7 +419,7 @@ class MyCustomComponentSpec(ComponentSpec): def __init__( self, - type: Optional[Type[artifact.Artifact]] = None, # pylint: disable=redefined-builtin + type: Optional[Type[artifact.Artifact]] = None, # noqa: A002 optional: bool = False, allow_empty: Optional[bool] = None, is_async: bool = False, diff --git a/tfx/v1/dsl/io/fileio.py b/tfx/v1/dsl/io/fileio.py index 6cb1e2f894..571fc62206 100644 --- a/tfx/v1/dsl/io/fileio.py +++ b/tfx/v1/dsl/io/fileio.py @@ -22,7 +22,7 @@ from tfx.dsl.io.fileio import makedirs from tfx.dsl.io.fileio import mkdir from tfx.dsl.io.fileio import NotFoundError -from tfx.dsl.io.fileio import open # pylint: disable=redefined-builtin +from tfx.dsl.io.fileio import open # noqa: A002 from tfx.dsl.io.fileio import PathType from tfx.dsl.io.fileio import remove from tfx.dsl.io.fileio import rename diff --git a/tfx/v1/dsl/placeholders/__init__.py b/tfx/v1/dsl/placeholders/__init__.py index e78707d137..68c20db186 100644 --- a/tfx/v1/dsl/placeholders/__init__.py +++ b/tfx/v1/dsl/placeholders/__init__.py @@ -16,7 +16,7 @@ from tfx.dsl.placeholder.placeholder import exec_property from tfx.dsl.placeholder.placeholder import execution_invocation -from tfx.dsl.placeholder.placeholder import input # pylint: disable=redefined-builtin +from tfx.dsl.placeholder.placeholder import input # noqa: A002 from tfx.dsl.placeholder.placeholder import output __all__ = [ From 0bed89b9c9f8b64fe55a8d04d8198dfba0529224 Mon Sep 17 00:00:00 2001 From: smokestacklightnin <125844868+smokestacklightnin@users.noreply.github.com> Date: Sat, 26 Oct 2024 19:22:53 -0700 Subject: [PATCH 12/16] Fix Ruff rule A001 --- .../input_resolution/canned_resolver_functions.py | 2 +- tfx/dsl/input_resolution/ops/test_utils.py | 2 +- tfx/dsl/io/fileio.py | 2 +- tfx/dsl/placeholder/artifact_placeholder.py | 2 +- .../mlmd_resolver/metadata_resolver.py | 12 ++++++------ 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/tfx/dsl/input_resolution/canned_resolver_functions.py b/tfx/dsl/input_resolution/canned_resolver_functions.py index 521fc55b42..f94543f2b1 100644 --- a/tfx/dsl/input_resolution/canned_resolver_functions.py +++ b/tfx/dsl/input_resolution/canned_resolver_functions.py @@ -744,7 +744,7 @@ def pick(channel: channel_types.BaseChannel, i: int, /): return _slice(channel, start=i, stop=(i + 1) or None, min_count=1) -def slice( # noqa: A002 +def slice( # noqa: A002, A001 channel: channel_types.BaseChannel, /, start: Optional[int] = None, diff --git a/tfx/dsl/input_resolution/ops/test_utils.py b/tfx/dsl/input_resolution/ops/test_utils.py index ba0ca08b61..5ed8e16828 100644 --- a/tfx/dsl/input_resolution/ops/test_utils.py +++ b/tfx/dsl/input_resolution/ops/test_utils.py @@ -54,7 +54,7 @@ class DummyArtifact(types.Artifact): # noqa: A002 def __init__( self, - id: Optional[str] = None, + id: Optional[str] = None, # noqa: A001, A002 uri: Optional[str] = None, create_time_since_epoch: Optional[int] = None, ): diff --git a/tfx/dsl/io/fileio.py b/tfx/dsl/io/fileio.py index e700191f62..7bd72fd244 100644 --- a/tfx/dsl/io/fileio.py +++ b/tfx/dsl/io/fileio.py @@ -33,7 +33,7 @@ def _get_filesystem(path) -> Type[filesystem.Filesystem]: .get_filesystem_for_path(path)) -def open(path: PathType, mode: str = 'r'): # noqa: A002 +def open(path: PathType, mode: str = 'r'): # noqa: A002, A001 """Open a file at the given path.""" return _get_filesystem(path).open(path, mode=mode) diff --git a/tfx/dsl/placeholder/artifact_placeholder.py b/tfx/dsl/placeholder/artifact_placeholder.py index 161ccea0f9..432bf9c9fa 100644 --- a/tfx/dsl/placeholder/artifact_placeholder.py +++ b/tfx/dsl/placeholder/artifact_placeholder.py @@ -23,7 +23,7 @@ _types = placeholder_base.types -def input(key: str) -> ArtifactPlaceholder: # noqa: A002 +def input(key: str) -> ArtifactPlaceholder: # noqa: A002, A001 """Returns a Placeholder that represents an input artifact. Args: diff --git a/tfx/orchestration/portable/input_resolution/mlmd_resolver/metadata_resolver.py b/tfx/orchestration/portable/input_resolution/mlmd_resolver/metadata_resolver.py index 553e8ec86f..8ecbecf5a4 100644 --- a/tfx/orchestration/portable/input_resolution/mlmd_resolver/metadata_resolver.py +++ b/tfx/orchestration/portable/input_resolution/mlmd_resolver/metadata_resolver.py @@ -305,7 +305,7 @@ def get_downstream_artifacts_by_artifact_ids( if store is None: raise ValueError('MetadataStore provided to MetadataResolver is None.') - artifact_ids_str = ','.join(str(id) for id in artifact_ids) + artifact_ids_str = ','.join(str(id) for id in artifact_ids) # noqa: A001 # If `max_num_hops` is set to 0, we don't need the graph traversal. if max_num_hops == 0: if not filter_query: @@ -370,7 +370,7 @@ def get_downstream_artifacts_by_artifact_ids( candidate_artifact_ids.update( visited_ids[metadata_resolver_utils.NodeType.ARTIFACT] ) - artifact_ids_str = ','.join(str(id) for id in candidate_artifact_ids) + artifact_ids_str = ','.join(str(id) for id in candidate_artifact_ids) # noqa: A001 # Send a call to metadata_store to get filtered downstream artifacts. artifacts = store.get_artifacts( list_options=mlmd.ListOptions( @@ -387,7 +387,7 @@ def get_downstream_artifacts_by_artifact_ids( artifact_id_to_artifact[id], artifact_type_by_id[artifact_id_to_artifact[id].type_id], ) - for id in visited_ids[metadata_resolver_utils.NodeType.ARTIFACT] + for id in visited_ids[metadata_resolver_utils.NodeType.ARTIFACT] # noqa: A001 if id in artifact_id_to_artifact ] if downstream_artifacts: @@ -594,7 +594,7 @@ def get_upstream_artifacts_by_artifact_ids( if store is None: raise ValueError('MetadataStore provided to MetadataResolver is None.') - artifact_ids_str = ','.join(str(id) for id in artifact_ids) + artifact_ids_str = ','.join(str(id) for id in artifact_ids) # noqa: A001 # If `max_num_hops` is set to 0, we don't need the graph traversal. if max_num_hops == 0: if not filter_query: @@ -662,7 +662,7 @@ def get_upstream_artifacts_by_artifact_ids( candidate_artifact_ids.update( visited_ids[metadata_resolver_utils.NodeType.ARTIFACT] ) - artifact_ids_str = ','.join(str(id) for id in candidate_artifact_ids) + artifact_ids_str = ','.join(str(id) for id in candidate_artifact_ids) # noqa: A001 # Send a call to metadata_store to get filtered upstream artifacts. artifacts = store.get_artifacts( list_options=mlmd.ListOptions( @@ -679,7 +679,7 @@ def get_upstream_artifacts_by_artifact_ids( artifact_id_to_artifact[id], artifact_type_by_id[artifact_id_to_artifact[id].type_id], ) - for id in visited_ids[metadata_resolver_utils.NodeType.ARTIFACT] + for id in visited_ids[metadata_resolver_utils.NodeType.ARTIFACT] # noqa: A001 if id in artifact_id_to_artifact ] if upstream_artifacts: From 2f05dd76922828984ac070dba9bd4b3316a51bac Mon Sep 17 00:00:00 2001 From: smokestacklightnin <125844868+smokestacklightnin@users.noreply.github.com> Date: Sat, 26 Oct 2024 19:25:14 -0700 Subject: [PATCH 13/16] Fix Ruff rule LOG --- tfx/utils/logging_utils_test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tfx/utils/logging_utils_test.py b/tfx/utils/logging_utils_test.py index b5f566236d..75fb7a45ab 100644 --- a/tfx/utils/logging_utils_test.py +++ b/tfx/utils/logging_utils_test.py @@ -49,9 +49,9 @@ def testDefaultSettings(self): def testOverrideSettings(self): """Ensure log overrides are set correctly.""" - config = logging_utils.LoggerConfig(log_root='path', log_level=logging.WARN, + config = logging_utils.LoggerConfig(log_root='path', log_level=logging.WARNING, pipeline_name='pipe', worker_name='wrk') self.assertEqual(config.log_root, 'path') - self.assertEqual(config.log_level, logging.WARN) + self.assertEqual(config.log_level, logging.WARNING) self.assertEqual(config.pipeline_name, 'pipe') self.assertEqual(config.worker_name, 'wrk') From c78305fd9be6e514163f6c02c78a6a0936b35700 Mon Sep 17 00:00:00 2001 From: smokestacklightnin <125844868+smokestacklightnin@users.noreply.github.com> Date: Sat, 26 Oct 2024 19:33:04 -0700 Subject: [PATCH 14/16] Convert `pylint: disable=unused-variable` to Ruff equivalent `noqa: F841` --- tfx/dsl/component/experimental/decorators_test.py | 2 +- .../component/experimental/decorators_typeddict_test.py | 2 +- tfx/dsl/control_flow/for_each_test.py | 8 ++++---- tfx/dsl/input_resolution/resolver_op_test.py | 2 +- tfx/experimental/templates/penguin/pipeline/pipeline.py | 8 ++++---- tfx/experimental/templates/taxi/pipeline/pipeline.py | 6 +++--- tfx/types/component_spec_test.py | 2 +- 7 files changed, 15 insertions(+), 15 deletions(-) diff --git a/tfx/dsl/component/experimental/decorators_test.py b/tfx/dsl/component/experimental/decorators_test.py index 8a2933904e..64d1bfcba3 100644 --- a/tfx/dsl/component/experimental/decorators_test.py +++ b/tfx/dsl/component/experimental/decorators_test.py @@ -418,7 +418,7 @@ def testDefinitionInClosureFails(self): 'the module level', ): @component - def my_component(): # pylint: disable=unused-variable + def my_component(): # noqa: F841 return None def testNonKwargFails(self): diff --git a/tfx/dsl/component/experimental/decorators_typeddict_test.py b/tfx/dsl/component/experimental/decorators_typeddict_test.py index f1c6a4222d..6ffe8b14b0 100644 --- a/tfx/dsl/component/experimental/decorators_typeddict_test.py +++ b/tfx/dsl/component/experimental/decorators_typeddict_test.py @@ -432,7 +432,7 @@ def testDefinitionInClosureFails(self): ): @component - def my_component(): # pylint: disable=unused-variable + def my_component(): # noqa: F841 return None def testNonKwargFails(self): diff --git a/tfx/dsl/control_flow/for_each_test.py b/tfx/dsl/control_flow/for_each_test.py index f3132ba752..9d9597fc22 100644 --- a/tfx/dsl/control_flow/for_each_test.py +++ b/tfx/dsl/control_flow/for_each_test.py @@ -88,8 +88,8 @@ def testForEach_LoopVariableNotUsed_Disallowed(self): with self.subTest('Not using at all'): with self.assertRaises(ValueError): a = A() - with for_each.ForEach(a.outputs['aa']) as aa: # pylint: disable=unused-variable - b = B() # aa is not used. # pylint: disable=unused-variable + with for_each.ForEach(a.outputs['aa']) as aa: # noqa: F841 + b = B() # aa is not used. # noqa: F841 with self.subTest('Source channel is not a loop variable.'): with self.assertRaises(ValueError): @@ -116,9 +116,9 @@ def testForEach_DifferentLoop_HasDifferentContext(self): a = A() b = B() with for_each.ForEach(a.outputs['aa']) as aa1: - c1 = C(aa=aa1, bb=b.outputs['bb']) # pylint: disable=unused-variable + c1 = C(aa=aa1, bb=b.outputs['bb']) # noqa: F841 with for_each.ForEach(a.outputs['aa']) as aa2: - c2 = C(aa=aa2, bb=b.outputs['bb']) # pylint: disable=unused-variable + c2 = C(aa=aa2, bb=b.outputs['bb']) # noqa: F841 context1 = dsl_context_registry.get().get_contexts(c1)[-1] context2 = dsl_context_registry.get().get_contexts(c2)[-1] diff --git a/tfx/dsl/input_resolution/resolver_op_test.py b/tfx/dsl/input_resolution/resolver_op_test.py index ef8db1f953..4478fee6f0 100644 --- a/tfx/dsl/input_resolution/resolver_op_test.py +++ b/tfx/dsl/input_resolution/resolver_op_test.py @@ -104,7 +104,7 @@ class ResolverOpTest(tf.test.TestCase): def testDefineOp_PropertyDefaultViolatesType(self): with self.assertRaises(TypeError): - class BadProperty(resolver_op.ResolverOp): # pylint: disable=unused-variable + class BadProperty(resolver_op.ResolverOp): # noqa: F841 str_prop = resolver_op.Property(type=str, default=42) def testOpCall_ReturnsOpNode(self): diff --git a/tfx/experimental/templates/penguin/pipeline/pipeline.py b/tfx/experimental/templates/penguin/pipeline/pipeline.py index 027f9c7142..9a2795aae2 100644 --- a/tfx/experimental/templates/penguin/pipeline/pipeline.py +++ b/tfx/experimental/templates/penguin/pipeline/pipeline.py @@ -65,13 +65,13 @@ def create_pipeline( components.append(schema_gen) # Performs anomaly detection based on statistics and data schema. - example_validator = tfx.components.ExampleValidator( # pylint: disable=unused-variable + example_validator = tfx.components.ExampleValidator( # noqa: F841 statistics=statistics_gen.outputs['statistics'], schema=schema_gen.outputs['schema']) components.append(example_validator) # Performs transformations and feature engineering in training and serving. - transform = tfx.components.Transform( # pylint: disable=unused-variable + transform = tfx.components.Transform( # noqa: F841 examples=example_gen.outputs['examples'], schema=schema_gen.outputs['schema'], preprocessing_fn=preprocessing_fn) @@ -125,7 +125,7 @@ def create_pipeline( absolute={'value': -1e-10}))) ]) ]) - evaluator = tfx.components.Evaluator( # pylint: disable=unused-variable + evaluator = tfx.components.Evaluator( # noqa: F841 examples=example_gen.outputs['examples'], model=trainer.outputs['model'], baseline_model=model_resolver.outputs['model'], @@ -135,7 +135,7 @@ def create_pipeline( # components.append(evaluator) # Pushes the model to a file destination if check passed. - pusher = tfx.components.Pusher( # pylint: disable=unused-variable + pusher = tfx.components.Pusher( # noqa: F841 model=trainer.outputs['model'], model_blessing=evaluator.outputs['blessing'], push_destination=tfx.proto.PushDestination( diff --git a/tfx/experimental/templates/taxi/pipeline/pipeline.py b/tfx/experimental/templates/taxi/pipeline/pipeline.py index 0da29ae8ea..043ad643dd 100644 --- a/tfx/experimental/templates/taxi/pipeline/pipeline.py +++ b/tfx/experimental/templates/taxi/pipeline/pipeline.py @@ -74,7 +74,7 @@ def create_pipeline( # components.append(schema_gen) # Performs anomaly detection based on statistics and data schema. - example_validator = tfx.components.ExampleValidator( # pylint: disable=unused-variable + example_validator = tfx.components.ExampleValidator( # noqa: F841 statistics=statistics_gen.outputs['statistics'], schema=schema_gen.outputs['schema']) # TODO(step 5): (Optional) Uncomment here to add ExampleValidator to the @@ -164,12 +164,12 @@ def create_pipeline( .PUSHER_SERVING_ARGS_KEY: ai_platform_serving_args } - pusher = tfx.extensions.google_cloud_ai_platform.Pusher(**pusher_args) # pylint: disable=unused-variable + pusher = tfx.extensions.google_cloud_ai_platform.Pusher(**pusher_args) # noqa: F841 else: pusher_args['push_destination'] = tfx.proto.PushDestination( filesystem=tfx.proto.PushDestination.Filesystem( base_directory=serving_model_dir)) - pusher = tfx.components.Pusher(**pusher_args) # pylint: disable=unused-variable + pusher = tfx.components.Pusher(**pusher_args) # noqa: F841 # TODO(step 6): Uncomment here to add Pusher to the pipeline. # components.append(pusher) diff --git a/tfx/types/component_spec_test.py b/tfx/types/component_spec_test.py index d154f30d0b..9d026af64e 100644 --- a/tfx/types/component_spec_test.py +++ b/tfx/types/component_spec_test.py @@ -67,7 +67,7 @@ class _BasicComponentSpec(ComponentSpec): class ComponentSpecTest(tf.test.TestCase): - # pylint: disable=unused-variable + # noqa: F841 def testComponentSpec_Empty(self): From 0c8cd754abf690b0bb773385bfc70befef2912d7 Mon Sep 17 00:00:00 2001 From: smokestacklightnin <125844868+smokestacklightnin@users.noreply.github.com> Date: Sat, 26 Oct 2024 19:39:40 -0700 Subject: [PATCH 15/16] Fix Ruff rule F841 --- tfx/utils/io_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tfx/utils/io_utils.py b/tfx/utils/io_utils.py index 0eaab2bba4..f76dd8c689 100644 --- a/tfx/utils/io_utils.py +++ b/tfx/utils/io_utils.py @@ -25,7 +25,7 @@ try: from tensorflow_metadata.proto.v0.schema_pb2 import Schema as schema_pb2_Schema # pylint: disable=g-import-not-at-top,g-importing-member -except ModuleNotFoundError as e: +except ModuleNotFoundError: schema_pb2_Schema = None # pylint: disable=invalid-name # Nano seconds per second. From bb24e15a8857b6995b49297d59bb13603a044a73 Mon Sep 17 00:00:00 2001 From: smokestacklightnin <125844868+smokestacklightnin@users.noreply.github.com> Date: Sat, 26 Oct 2024 19:40:50 -0700 Subject: [PATCH 16/16] Fix Ruff rule F811 --- tfx/dsl/placeholder/placeholder_base.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tfx/dsl/placeholder/placeholder_base.py b/tfx/dsl/placeholder/placeholder_base.py index 07a792a7d7..3c8d1169c7 100644 --- a/tfx/dsl/placeholder/placeholder_base.py +++ b/tfx/dsl/placeholder/placeholder_base.py @@ -45,7 +45,8 @@ # where ComponentSpecIntf is an interface that gives access to the few things # that Placeholder::encode() needs from the spec. TODO(pke) Implement. # TODO(b/191610358): Reduce the number of circular type-dependencies. -types = Any # To resolve circular dependency caused by type annotations. +# To resolve circular dependency caused by type annotations. +types = Any # noqa: F811 # TODO(b/190409099): Support RuntimeParameter. ValueType = Union[int, float, str, bool]