Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nested Function Definitions Introduce Whitespace #196

Closed
kevinastone opened this issue May 8, 2018 · 5 comments
Closed

Nested Function Definitions Introduce Whitespace #196

kevinastone opened this issue May 8, 2018 · 5 comments

Comments

@kevinastone
Copy link

Operating system: macOS
Python version: 3.6
Black version: black, version 18.4a4
Does also happen on master: Yes

Black is persistent about function declarations having vertical whitespace even though pep8 only prescribes them for top-level and class method declarations.

Examples:
https://github.com/python/cpython/blob/master/Lib/contextlib.py#L600-L618

Example

from functools import wraps


def my_decorator(func=None, a=1, b=2):
    def decorator(func):
        @wraps(func)
        def wrapper(*args, **kwargs):
            kwargs.update(dict(a=a, b=b))
            return func(*args, **kwargs)
        return wrapper

    if func:
        return decorator(func)
    return decorator

Result

kstone@mbp ~> black --diff .
--- test.py  (original)
+++ test.py  (formatted)
@@ -1,14 +1,18 @@
 from functools import wraps
 
 
 def my_decorator(func=None, a=1, b=2):
+
     def decorator(func):
+
         @wraps(func)
         def wrapper(*args, **kwargs):
             kwargs.update(dict(a=a, b=b))
             return func(*args, **kwargs)
+
         return wrapper

     if func:
         return decorator(func)
     return decorator
 
reformatted test.py
@ambv
Copy link
Collaborator

ambv commented May 8, 2018

Noted. I'm not sure how easy this is going to be to change since Black doesn't keep track whether a function definition is internal to a function definition or to a class definition. Spacing it out always doesn't hurt, especially when differentiating where the inner function ends. I guess your example wouldn't look as weird if there were docstrings involved.

I will look into improving this.

@kevinastone
Copy link
Author

kevinastone commented May 8, 2018

I think black would be better to track module/class/function context. The current 1 if depth else 2 feels brittle.

I can submit a PR to see how much complication this introduces.

@ambv
Copy link
Collaborator

ambv commented May 29, 2018

Resolution: nested function definitions will introduce whitespace between docstrings and other instructions but will not introduce leading empty lines if they are the start of the wrapping functions body.

The diff from your example will look like this now:

--- /tmp/asd.py  (original)
+++ /tmp/asd.py  (formatted)
@@ -5,10 +5,11 @@
     def decorator(func):
         @wraps(func)
         def wrapper(*args, **kwargs):
             kwargs.update(dict(a=a, b=b))
             return func(*args, **kwargs)
+
         return wrapper

     if func:
         return decorator(func)
     return decorator

I think this is reasonable and does what you wanted.

@rouge8
Copy link
Contributor

rouge8 commented May 29, 2018

So that would format the example in #144 (comment) as:

def decorator(f):
    """A decorator."""

    @wraps(f)
    def inner():
        return f()

    return inner

?

@ambv ambv closed this as completed in 2057bf6 May 29, 2018
@ambv
Copy link
Collaborator

ambv commented May 29, 2018

@rouge8, yes. While it's not compatible with PEP 257 to the letter, I think it's close enough in spirit. I want inner functions to be delimited and there's no reason to hug them to a docstring. Other statements after a function docstring don't generate an empty line.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants