add magic method XCom docstrings D105

[okirialbert]

---

^ Add meaningful description above Read the Pull Request Guidelines for more information. In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed. In case of a new dependency, check compliance with the ASF 3rd Party License Policy. In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

[okirialbert]

Add XCom magic method docstrings #37523

[okirialbert]

I was striving for better readability by including detailed function descriptions but now I see how it can be misleading in the event of future changes.

[uranusjr]

Magic methods already have defined semantics and documenting them is entirely missing the point. If a tool requires you to do it, it is not a good tool.

[potiuk]

Magic methods already have defined semantics and documenting them is entirely missing the point. If a tool requires you to do it, it is not a good tool.

Yep. Agree with @uranusjr - we shoudl NOT be documenting those (dunder) methods at all.

[Taragolis]

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.

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?

[potiuk]

As discussed in https://github.com/apache/airflow/pull/38452 - we remove the D105 rule from our checks.