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
>
>
