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