+1 On Fri, Nov 29, 2024 at 12:28 AM Jarek Potiuk <ja...@potiuk.com> wrote:
> FYI: This is how it will look like when SIM300 is applied: > https://github.com/apache/airflow/pull/44466 > > On Thu, Nov 28, 2024 at 11:23 PM Jens Scheffler <j_scheff...@gmx.de.invalid > > > wrote: > > > This is nto a vote but still +1 :-D > > > > On 28.11.24 22:48, Ash Berlin-Taylor wrote: > > > +10, it’s defence against a problem that is impossible in Python, _and_ > > it’s counter to how pytest wants it. > > > > > >> On 28 Nov 2024, at 21:46, Ferruzzi, Dennis > <ferru...@amazon.com.INVALID> > > wrote: > > >> > > >> +0.5 LOL I don't particularly care which order they go in, but I > do > > love standardizing the codebase and that IS how pytest reports it, so I > say > > let's go for it. > > >> > > >> > > >> - ferruzzi > > >> > > >> > > >> ________________________________ > > >> From: Jarek Potiuk <ja...@potiuk.com> > > >> Sent: Thursday, November 28, 2024 11:16 AM > > >> To: dev@airflow.apache.org > > >> Subject: RE: [EXT] [DISCUSS] Avoid Yoda conditions in our code > > >> > > >> 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. > > >> > > >> > > >> > > >> OK. And now let the flamewar starts ;) > > >> > > >> On Thu, Nov 28, 2024 at 8:11 PM Jarek Potiuk <ja...@potiuk.com> > wrote: > > >> > > >>> Hello here, > > >>> > > >>> Following from slack discussion: > > >>> > > https://apache-airflow.slack.com/archives/C06K9Q5G2UA/p1732816777850809 > > >>> > > >>> While responding to > > https://github.com/apache/airflow/discussions/44455 > > >>> (really nice that Astral people are using Airflow to test new ruff > > rules) I > > >>> noticed that we have a number of cases where we use > > >>> > > >>> assert EXPECTED_CONSTANT == value tested > > >>> > > >>> rather than > > >>> > > >>> assert value tested == EXPECTED_CONSTANT > > >>> > > >>> I just learned that The way to write this condition is called "Yoda > > >>> condition" :) - for obvious reasons :) > > >>> > > >>> I know where it comes from - > > >>> https://wiki.c2.com/?CompareConstantsFromTheLeft - because there is > > (was) > > >>> easy to make mistake and have `=` instead of `==`, but this is from C > > and > > >>> not from Python. In Python it literally makes no sense to prevent > this, > > >>> because it leads to a syntax error: > > >>> > > >>>>>> x = 10 > > >>>>>> assert x = 9 > > >>> File "<stdin>", line 1 > > >>> assert x = 9 > > >>> ^ > > >>> SyntaxError: invalid syntax > > >>> Also Pytest is very clear in their "assert rewritten messages" that > the > > >>> "right" side is expected > > >>> > > >>> tests/jobs/test_scheduler_job.py:1695 > > >>> (TestSchedulerJob.test_execute_task_instances_limit_second_executor) > > >>> 7 != 6 > > >>> > > >>> Expected :6 > > >>> Actual :7 > > >>> <Click to see difference> > > >>> > > >>> test_scheduler_job.py:1736: in > > >>> test_execute_task_instances_limit_second_executor > > >>> assert 7 == res > > >>> E assert 7 == 6 > > >>> > > >>> IMHO - using this `assert CONSTANT == value` - is a bit of a "cargo > > cult" > > >>> - we are using it without realising that it is no use any more and > only > > >>> introduces confusion. > > >>> > > >>> Quick check shows that we have 398 "yoda asserts" in our tests, so > > >>> apparently the cult is pretty strong :D. > > >>> > > >>> I would say we should make Yoda conditions forbidden via the > > >>> https://docs.astral.sh/ruff/rules/yoda-conditions/ rule and fix all > > those > > >>> Yoda conditions to be "non-Yoda" conditions (including all the > > asserts). > > >>> > > >>> WDYT? > > >>> > > >>> 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 > > > > >
