Multiple context managers in a single with are not pretty

[erichurkman]

Some testing frameworks like django-mock-querysets make liberal use of multiple context managers to handle applying mock calls through monkey patching and using __exit__ to unwind those monkey patches. black makes multiple context manager calls... difficult... to read & review.

Operating system: macOS
Python version: 3.6
Black version: 18.6b4
Does also happen on master:

Examples:

-    with patch('myapp.models.Jawn.objects', jawns), \
-         patch('myapp.models.Thing.objects', things):
+    with patch("myapp.models.Jawn.objects", jawns), patch(
+        "myapp.models.Thing.objects", things
+    ):

Or even more ear bleeding:

-    with patch("myapp.models.Jawn.objects", jawns), \
-         patch("myapp.models.Thing.objects", MockSet(a_thing)), \
-         patch("accounting.models.Account.objects", MockSet(chocula)):
+    with patch("myapp.models.Jawn.objects", jawns), patch(
+        "myapp.models.Thing.objects", MockSet(a_thing)
+    ), patch("accounting.models.Account.objects", MockSet(chocula)):

Unfortunately with statements do not allow using parenthesis. There is an equally ugly change that at least avoids weird hanging arguments, but requires code change and hides the purpose of the context manager and is Python 3.6+.

with contextlib.ExitStack() as patches:
    patches.enter_context(patch("myapp.models.Jawn.objects", jawns))
    patches.enter_context(patch("myapp.models.Thing.objects", things))

[cxong]

Well, there is another alternative, since you can use extra parentheses:

with (
    open('foo1', 'w')
) as f1, (
    open('foo2', 'r')
) as f2:
    f1.read()
    f2.read()

This is also ugly but in a different way.
More reading here: https://bugs.python.org/issue12782

The ExitStack workaround doesn't produce the same AST does it?

[carljm]

Yeah, ExitStack is a code change, not formatting; that transformation is out of the question for Black.

So is using backslash continuations, as a Black design choice.

I think the formatting shown by @cxong is pretty reasonable; it might be worth exploring whether Black could wrap context manager expressions in extra parens when there are multiple in a single with statement.

[ems-ja]

Would prefer formatting shown by @cxong, too.

[theelous3]

Really? @cxong's example is unreadable. If a human being gave you this in a code review or interview, you would have them removed from the building.

It's clear that the current behavior isn't wonderful, but it's light-years better than anything proposed thus far.

[ambv]

This is unlikely to change unless Python enhances its unfortunate with grammar in this case. It really needs to allow wrapping everything after with in optional parentheses, which would look like this:

with (
  open('foo1', 'w') as f1,
  open('foo2', 'w') as f2,
):
    ...

There is precedent in imports:

from os import (
    path as p,
    environ as e,
    system as s,
)

[kaste]

I just had this today.

In

        with when(rex).waggle().thenReturn('Yup'), \
             when(mox).waggle().thenReturn('Nope'):  
            assert rex.waggle() == 'Yup'
            assert mox.waggle() == 'Nope'

Out

        with when(rex).waggle().thenReturn('Yup'), when(mox).waggle().thenReturn(
            'Nope'
        ):
             assert rex.waggle() == 'Yup'
             assert mox.waggle() == 'Nope'

(That's actual code, it has funny names bc it comes from a test suite.)

A grammar change is a bit unlikely, and black is probably a tool for todays
python anyway. Since the output is just not okay, we would have to use
# fmt off commonly here. Maybe black could see the backslash in the original
code and turn itself off.

(Also note that backslahes for with statements are probably the
only valid usecase for backslashes in todays python. They are basically
recommended in PEP8 https://www.python.org/dev/peps/pep-0008/?#maximum-line-length.)

EDIT: blacks output is actually now:

        with when(rex).waggle().thenReturn('Yup'), when(
            mox
        ).waggle().thenReturn(
            'Nope'
        ):
			...

[theelous3]

I think the refusal to allow backslashes at all is absolutist and this is a clear demonstration of why.

When the best solution for the tool is to turn it off in its specific domain, there is a problem

[ambv]

Note: @erichurkman, it took me a while but I changed my mind on this. We will be tracking the solution as part of #664.