Fix instability with await fluent style

[konstin]

Fix an instability where await was followed by a breaking fluent style expression:

test_data = await (
    Stream.from_async(async_data)
    .flat_map_async()
    .map()
    .filter_async(is_valid_data)
    .to_list()
)

Note that this technically a minor style change (see ecosystem check)

[konstin]

This is arguably correct for ruff

[zanieb]

This seems like really weird formatting. Why is this correct?

[konstin]

Note that we don't expand, we just keep

async def main():
    await asyncio.sleep(1)  # Hello
    await (
        asyncio.sleep(1)  # Hello
    )

as-is

[zanieb]

Okay that's good to know. I guess that's okay. A little weird but fits with previous choices?

[charliermarsh]

Why does this work with IfBreaks?

[charliermarsh]

In yield we use Parenthesize::Optional...

[konstin]

I tried that too for that same reason, but it introduced new black deviations:

 # Remove brackets for short coroutine/task
 async def main():
-    await asyncio.sleep(1)
+    await (asyncio.sleep(1))
 
 
 async def main():
-    await asyncio.sleep(1)
+    await (asyncio.sleep(1))
 
 
 async def main():
-    await asyncio.sleep(1)
+    await (asyncio.sleep(1))

[github-actions]

ruff-ecosystem results

Formatter (stable)

ℹ️ ecosystem check detected format changes. (+9 -5 lines in 1 file in 41 projects)

RasaHQ/rasa (+9 -5 lines across 1 file)

rasa/server.py~L880

 
         try:
             async with app.ctx.agent.lock_store.lock(conversation_id):
-                tracker = await app.ctx.agent.processor.fetch_tracker_and_update_session(
-                    conversation_id
+                tracker = await (
+                    app.ctx.agent.processor.fetch_tracker_and_update_session(
+                        conversation_id
+                    )
                 )
 
                 output_channel = _get_output_channel(request, tracker)

rasa/server.py~L931

 
         try:
             async with app.ctx.agent.lock_store.lock(conversation_id):
-                tracker = await app.ctx.agent.processor.fetch_tracker_and_update_session(
-                    conversation_id
+                tracker = await (
+                    app.ctx.agent.processor.fetch_tracker_and_update_session(
+                        conversation_id
+                    )
                 )
                 output_channel = _get_output_channel(request, tracker)
                 if intent_to_trigger not in app.ctx.agent.domain.intents:

Formatter (preview)

ℹ️ ecosystem check detected format changes. (+9 -5 lines in 1 file in 41 projects)

RasaHQ/rasa (+9 -5 lines across 1 file)

ruff format --preview

rasa/server.py~L880

 
         try:
             async with app.ctx.agent.lock_store.lock(conversation_id):
-                tracker = await app.ctx.agent.processor.fetch_tracker_and_update_session(
-                    conversation_id
+                tracker = await (
+                    app.ctx.agent.processor.fetch_tracker_and_update_session(
+                        conversation_id
+                    )
                 )
 
                 output_channel = _get_output_channel(request, tracker)

rasa/server.py~L931

 
         try:
             async with app.ctx.agent.lock_store.lock(conversation_id):
-                tracker = await app.ctx.agent.processor.fetch_tracker_and_update_session(
-                    conversation_id
+                tracker = await (
+                    app.ctx.agent.processor.fetch_tracker_and_update_session(
+                        conversation_id
+                    )
                 )
                 output_channel = _get_output_channel(request, tracker)
                 if intent_to_trigger not in app.ctx.agent.domain.intents:

[charliermarsh]

Should this be IfBreaksOrIfRequired?

[konstin]

That one would regress asyncio.gather

 # Long lines
 async def main():
-    await asyncio.gather(
-        asyncio.sleep(1),
-        asyncio.sleep(1),
-        asyncio.sleep(1),
-        asyncio.sleep(1),
-        asyncio.sleep(1),
-        asyncio.sleep(1),
-        asyncio.sleep(1),
+    await (
+        asyncio.gather(
+            asyncio.sleep(1),
+            asyncio.sleep(1),
+            asyncio.sleep(1),
+            asyncio.sleep(1),
+            asyncio.sleep(1),
+            asyncio.sleep(1),
+            asyncio.sleep(1),
+        )
     )

(i'm happy too implement a different solution btw, this was just the solution that i found to work best by trial and error)

[charliermarsh]

Ahh yeah, I agree that that's worse. I feel like I need to have a better understanding of why these Parenthesize values are leading to these changes before I'd be comfortable approving. Why does IfBreaks break that way? Will using IfBreaks ever lead to syntax errors?

[konstin]

No, that's handled by OptionalParentheses::Multiline

[konstin]

I'll make some better examples why we get this behavior

[charliermarsh]

Thank you, I'm trying to make sure I play Micha and really understand the impact of formatting changes before approving.

[konstin]

From the perspective of the await, it needs OptionalParentheses::Never because the child is already parenthesized, and we're preferring those parentheses over adding them around the await. For the await's child, we have four options:

• Optional: Add parentheses if the child breaks, which we want e.g. for asyncio.gather. Keeps the child's parentheses from the source code, which we want to remove.
• IfBreaks: Add parentheses if the child breaks, which we want e.g. for asyncio.gather. Discards child parentheses, which we want.
• IfRequired: Discards child parentheses, which we want. Also discards them when the child breaks but has its own inner parentheses, which would lead to e.g.

  await asyncio.gather(
      fut1,
      fut2,
  )

  , which don't want.
• IfBreaksOrIfRequired: Special case for return type annotations.

I added comments to the parentheses types to make them better understandable in the future