+1 Agree, I do :-) On Thu, Nov 28, 2024 at 9:12 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. >
