+1 for this! This is how pytest doc https://docs.pytest.org/en/stable/how-to/assert.html suggests.
Best, Wei > On Nov 29, 2024, at 8:28 AM, Kaxil Naik <kaxiln...@gmail.com> wrote: > > Same as slack, prefer it on right. Right is right 😆 > > On Thu, 28 Nov 2024 at 23:48, Hussein Awala <huss...@awala.fr> wrote: > >> +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 >>>> >>>> >>> >>
