tensorflow / tfx

TFX is an end-to-end platform for deploying production ML pipelines
https://tensorflow.github.io/tfx/
Apache License 2.0
2.11k stars 707 forks source link

Abstract base classes have incorrectly decorated methods #6943

Open smokestacklightnin opened 1 week ago

smokestacklightnin commented 1 week ago

If the bug is related to a specific library below, please raise an issue in the respective repo directly: TFX

System information

Describe the current behavior

There are currently violations of Ruff rules B024 and B027, which have to do with incorrectly decorated class methods/properties. Fixing them currently causes errors (See #6942), so that issue should be dealt with before this one.

Describe the expected behavior

See python docs for the correct way to make methods and properties abstract.

Other info / logs

The code violating B024 and B027 is listed below:

tfx/orchestration/portable/base_executor_operator.py:87:3: B027 `BaseExecutorOperator.handle_stop` is an empty method in an abstract base class, but has no abstract decorator
   |
85 |       return self
86 |   
87 |     def handle_stop(self) -> None:
   |  ___^
88 | |     """Executor Operator specific logic to clean up after it is stopped."""
89 | |     pass
   | |________^ B027
   |

tfx/tools/cli/handler/dag_runner_patcher.py:59:3: B027 `DagRunnerPatcher._before_run` is an empty method in an abstract base class, but has no abstract decorator
   |
57 |       self._call_real_run = call_real_run
58 |   
59 |     def _before_run(self, runner: tfx_runner.TfxRunner,
   |  ___^
60 | |                   pipeline: Union[pipeline_pb2.Pipeline, tfx_pipeline.Pipeline],
61 | |                   context: MutableMapping[str, Any]) -> None:
62 | |     pass
   | |________^ B027
63 |   
64 |     def _after_run(self, runner: tfx_runner.TfxRunner,
   |

tfx/tools/cli/handler/dag_runner_patcher.py:64:3: B027 `DagRunnerPatcher._after_run` is an empty method in an abstract base class, but has no abstract decorator
   |
62 |       pass
63 |   
64 |     def _after_run(self, runner: tfx_runner.TfxRunner,
   |  ___^
65 | |                  pipeline: Union[pipeline_pb2.Pipeline, tfx_pipeline.Pipeline],
66 | |                  context: MutableMapping[str, Any]) -> None:
67 | |     pass
   | |________^ B027
68 |   
69 |     @abc.abstractmethod
   |

tfx/types/system_artifacts.py:24:7: B024 `SystemArtifact` is an abstract base class, but it has no abstract methods
   |
24 | class SystemArtifact(abc.ABC):
   |       ^^^^^^^^^^^^^^ B024
25 |   """TFX system artifact base class.
   |

tfx/types/system_executions.py:24:7: B024 `SystemExecution` is an abstract base class, but it has no abstract methods
   |
24 | class SystemExecution(abc.ABC):
   |       ^^^^^^^^^^^^^^^ B024
25 |   """TFX system execution base class.
   |

Found 5 errors.
janasangeetha commented 1 week ago

Hi @smokestacklightnin, Thank you for reporting. I'll take a look and provide an update here.

lego0901 commented 2 days ago

QQ. How could you see those Ruff errors?

smokestacklightnin commented 2 days ago

QQ. How could you see those Ruff errors?

Using the ruff check command

lego0901 commented 2 days ago

Hmm.. My ruff check shows different errors like F401, and there is no B024 and B027. Could you let me know how to reproduce your output?

smokestacklightnin commented 2 days ago

This on example of a command I used on the master branch:

$ ruff check tfx --exclude tfx/examples/airflow_workshop/taxi/notebooks/*.ipynb --select B

from the root project directory

lego0901 commented 2 days ago

Great! Now I see the message. Thanks a lot!