+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
