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