Ok. Seems we have a consensus :) On Mon, Mar 25, 2024 at 7:13 AM Wei Lee <[email protected]> wrote:
> +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 <[email protected]> > 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 > <[email protected]> > > 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 <[email protected]> > >> Sent: Wednesday, March 20, 2024 5:51:43 AM > >> To: [email protected] > >> 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 <[email protected] > > > >> 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 <[email protected] > >> .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 < > >> [email protected]> > >>>>> 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 <[email protected]> > >> 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: [email protected] > >> For additional commands, e-mail: [email protected] > >> > >> > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [email protected] > For additional commands, e-mail: [email protected] > >
