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