+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