Yes, I agree. I just looked at the ruff , I think we have more to fix in the coming days :)
https://github.com/astral-sh/ruff/pull/14661 https://github.com/astral-sh/ruff/pull/14611 On Fri, Nov 29, 2024 at 7:06 AM Jarek Potiuk <ja...@potiuk.com> wrote: > > Seeing the universal support - I progress with > https://github.com/apache/airflow/pull/44466 -> Note it's not only for > asserts, there are a number of other places where comparison was not RIGHT > (pun intended).. > > > On Fri, Nov 29, 2024 at 7:50 AM Pavankumar Gopidesu <gopidesupa...@gmail.com> > wrote: > > > +1 agree :) > > > > Regards, > > Pavan > > > > > > On Fri, Nov 29, 2024, 06:37 Amogh Desai <amoghdesai....@gmail.com> wrote: > > > > > Vote +1 for this, I do. (Sorry couldn't help it) > > > > > > It's nice that there is some automation to do this. It was always > > slightly > > > annoying to me > > > coming from a different programming language background > > > > > > Thanks & Regards, > > > Amogh Desai > > > > > > > > > On Fri, Nov 29, 2024 at 12:00 PM Shahar Epstein <sha...@apache.org> > > wrote: > > > > > > > +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. > > > > > > > > > > > > > > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@airflow.apache.org For additional commands, e-mail: dev-h...@airflow.apache.org
