Taragolis commented on PR #37602: URL: https://github.com/apache/airflow/pull/37602#issuecomment-2001951402
My 2c, there is always depends on. E.g. why on earth we required to create `__str__` and `__repr__` for the classes: - is this just for beatify? - Or it has some specific cases, e.g. https://github.com/apache/airflow/blob/c0b849ad2b3f7015f7cb2a45aefd1fa3828bda31/airflow/models/xcom_arg.py#L277-L286 `__repr__` might have a specific meaning for the serialisation, e.g. ```python class Foo: ... class Bar: def __repr__(self): return "BarSentinel" ``` e.g. `Foo` will return a new value each time it created `<__main__.Foo object at 0x1011e5610>` or in another time it would be `<__main__.Foo object at 0x101065610>`. So if this object might use into the serialisation, then it will be the reason why we overwrite SerializedDag every time, and simple "String representation for deterministic output" might help understand why it required to set this method. The problem here that for some methods you have to define, e.g. if you subclassing of `collections.abc`, and that is pretty clear why you have to define and docstring in this case redundant I have not clear vision, should we define it for all classes (rule enable) or it should remain on conscience and responsibility of contributor/reviewers (rule disable). And there other question, do we have an idea why we originally (3 ago) select this rules https://github.com/apache/airflow/issues/10742 . For example Personally I thought it is redundant to also enable `D107 | Missing docstring in __init__`, so maybe better review and discuss this rules again and remove redundant one? -- 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]
