+1 for removing this rule. If docstring is needed for some cases, we can still 
do that or add comments to PRs.

Best,
Wei

> On Mar 25, 2024, at 1:31 PM, Amogh Desai <amoghdesai....@gmail.com> wrote:
> 
> After reading the emails in this thread, I too think that this rule is not
> generally very useful, but we should have
> it for cases which are special (where we are overriding some magic method
> to do something different or more
> than its reserved meaning)
> 
> +1 for removal of the rule, with special cases being handled separately.
> 
> Thanks & Best Regards,
> Amogh Desai
> 
> On Wed, Mar 20, 2024 at 11:46 PM Oliveira, Niko <oniko...@amazon.com.invalid>
> wrote:
> 
>> I'm -1 to enabling D105
>> 
>> 
>> I don't think it will lead to helpful documentation. I think for the rare
>> cases it is required it can left up to the developer or caught in PR review.
>> 
>> Cheers,
>> Niko
>> 
>> ________________________________
>> From: Vincent Beck <vincb...@apache.org>
>> Sent: Wednesday, March 20, 2024 5:51:43 AM
>> To: dev@airflow.apache.org
>> Subject: RE: [EXTERNAL] [COURRIEL EXTERNE] [DISCUSS] Applying D105 rule
>> for our codebase ("undocumented magic methods") ?
>> 
>> CAUTION: This email originated from outside of the organization. Do not
>> click links or open attachments unless you can confirm the sender and know
>> the content is safe.
>> 
>> 
>> 
>> AVERTISSEMENT: Ce courrier électronique provient d’un expéditeur externe.
>> Ne cliquez sur aucun lien et n’ouvrez aucune pièce jointe si vous ne pouvez
>> pas confirmer l’identité de l’expéditeur et si vous n’êtes pas certain que
>> le contenu ne présente aucun risque.
>> 
>> 
>> 
>> +1 for not enforcing as well. Let's leave to maintainers the flexibility
>> to chose whether a given method should be documented.
>> 
>> On 2024/03/20 08:28:51 Ash Berlin-Taylor wrote:
>>> I'm for not enforcing this rule - as others have said its very unlikely
>> to result in more useful docs for developers or end users.
>>> 
>>> -asg
>>> 
>>> On 20 March 2024 08:12:40 GMT, Andrey Anshin <andrey.ans...@taragol.is>
>> wrote:
>>>> ±0 from my side
>>>> 
>>>> Maybe we have to review all current methods which do not follow this
>> rule
>>>> to find a really useful meaning, and do not enforce (disable it).
>>>> So for avoid unnecessary changes we might close
>>>> https://github.com/apache/airflow/issues/37523 and remove/mark
>> completed
>>>> into the https://github.com/apache/airflow/issues/10742
>>>> 
>>>> 
>>>> 
>>>> 
>>>> 
>>>> 
>>>> On Wed, 20 Mar 2024 at 10:41, Pankaj Koti <pankaj.k...@astronomer.io
>> .invalid>
>>>> wrote:
>>>> 
>>>>> +1 to what Aritra is saying.
>>>>> 
>>>>> 
>>>>> Best regards,
>>>>> 
>>>>> *Pankaj Koti*
>>>>> Senior Software Engineer (Airflow OSS Engineering team)
>>>>> Location: Pune, Maharashtra, India
>>>>> Timezone: Indian Standard Time (IST)
>>>>> Phone: +91 9730079985
>>>>> 
>>>>> 
>>>>> On Wed, Mar 20, 2024 at 12:05 PM Aritra Basu <
>> aritrabasu1...@gmail.com>
>>>>> wrote:
>>>>> 
>>>>>> I'm in general not a huge fan of documenting for the sake of
>> documenting,
>>>>>> so I'd be in agreement of not enforcing it via code but rather be
>>>>> enforced
>>>>>> by the reviewers in cases they believe certain methods need
>> documenting.
>>>>>> 
>>>>>> --
>>>>>> Regards,
>>>>>> Aritra Basu
>>>>>> 
>>>>>> On Wed, Mar 20, 2024, 9:39 AM Jarek Potiuk <ja...@potiuk.com>
>> wrote:
>>>>>> 
>>>>>>> Hey here,
>>>>>>> 
>>>>>>> I wanted to quickly poll what people think about applying the
>>>>>>> https://docs.astral.sh/ruff/rules/undocumented-magic-method/
>> rule in
>>>>> our
>>>>>>> codebase. There are many uncontroversial rules - but that one is
>>>>> somewhat
>>>>>>> more controversial than others.
>>>>>>> 
>>>>>>> See
>>>>> https://github.com/apache/airflow/pull/37602#issuecomment-2001951402
>>>>>>> and
>>>>>>> 
>>>>>> 
>>>>> 
>> https://github.com/apache/airflow/pull/38277#pullrequestreview-1945745542
>>>>>>> for example
>>>>>>> 
>>>>>>> I think that even in the ruff example, but also in many cases
>> requiring
>>>>>> to
>>>>>>> document the methods will lead to rather useless documentation:
>>>>>>> 
>>>>>>> class Cat(Animal):
>>>>>>>    def __str__(self) -> str:
>>>>>>>        """Return a string representation of the cat."""
>>>>>>>        return f"Cat: {self.name}"
>>>>>>> 
>>>>>>> There is IMHO very little value in having such documentation. It
>> might
>>>>> be
>>>>>>> useful in some cases where we have a really good reason to add
>> such a
>>>>>> magic
>>>>>>> method and it is important to document it, but in many cases - the
>>>>>>> documentation will be just documenting what the magic method
>> already
>>>>>>> explains well (like the case above).
>>>>>>> 
>>>>>>> This actually reminds me the early days of java documentation
>> where
>>>>>> javadoc
>>>>>>> looks more or less like this:
>>>>>>> 
>>>>>>> "Paints the object"
>>>>>>> func paint()
>>>>>>> 
>>>>>>> "Repaints the object"
>>>>>>> fund repaint()
>>>>>>> 
>>>>>>> However - maybe I am wrong :). Maybe it's worth documenting those
>>>>> methods
>>>>>>> in bulk, even if in many cases it will not bring much value?
>>>>>>> 
>>>>>>> WDYT ? Should we mandate documenting it - or leave up to the
>> author to
>>>>>>> document it in case it feels like it is needed?
>>>>>>> 
>>>>>>> J.
>>>>>>> 
>>>>>> 
>>>>> 
>>> 
>> 
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscr...@airflow.apache.org
>> For additional commands, e-mail: dev-h...@airflow.apache.org
>> 
>> 


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@airflow.apache.org
For additional commands, e-mail: dev-h...@airflow.apache.org

Reply via email to