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
