Blank line between nested and function def in stub files. #3862

konstin · 2023-09-08T14:51:21Z

Description

The idea behind this change is that we stop looking into previous body to determine if there should be a blank before a function or class definition.

Input:

import sys

if sys.version_info > (3, 7):
    class Nested1:
        assignment = 1
        def function_definition(self): ...
    def f1(self) -> str: ...
    class Nested2:
        def function_definition(self): ...
        assignment = 1
    def f2(self) -> str: ...

if sys.version_info > (3, 7):
    def nested1():
        assignment = 1
        def function_definition(self): ...
    def f1(self) -> str: ...
    def nested2():
        def function_definition(self): ...
        assignment = 1
    def f2(self) -> str: ...

Stable style

import sys

if sys.version_info > (3, 7):
    class Nested1:
        assignment = 1
        def function_definition(self): ...

    def f1(self) -> str: ...

    class Nested2:
        def function_definition(self): ...
        assignment = 1
    def f2(self) -> str: ...

if sys.version_info > (3, 7):
    def nested1():
        assignment = 1
        def function_definition(self): ...

    def f1(self) -> str: ...
    def nested2():
        def function_definition(self): ...
        assignment = 1
    def f2(self) -> str: ...

In the stable formatting, we have a blank line sometimes, not depending on the previous statement on the same level, but on the last (potentially nested) statement in the previous body.

#2783/#3564 fixes this for classes in preview style:

import sys

if sys.version_info > (3, 7):
    class Nested1:
        assignment = 1
        def function_definition(self): ...

    def f1(self) -> str: ...

    class Nested2:
        def function_definition(self): ...
        assignment = 1

    def f2(self) -> str: ...

if sys.version_info > (3, 7):
    def nested1():
        assignment = 1
        def function_definition(self): ...

    def f1(self) -> str: ...
    def nested2():
        def function_definition(self): ...
        assignment = 1
    def f2(self) -> str: ...

This PR additionally fixes this for function definitions:

if sys.version_info > (3, 7):
    if sys.platform == "win32":
        assignment = 1
        def function_definition(self): ...

    def f1(self) -> str: ...
    if sys.platform != "win32":
        def function_definition(self): ...
        assignment = 1

    def f2(self) -> str: ...

if sys.version_info > (3, 8):
    if sys.platform == "win32":
        assignment = 1
        def function_definition(self): ...

    class F1: ...
    if sys.platform != "win32":
        def function_definition(self): ...
        assignment = 1

    class F2: ...

You can see the effect of this change on typeshed in konstin/typeshed#1. As baseline, the preview mode changes without this PR are at konstin/typeshed#2.

Closes #3861

Checklist - did you ...

Add an entry in CHANGES.md if necessary?
Add / update tests if necessary?
Add new / update outdated documentation?

The idea behind this change is that we stop looking into previous body to determine if there should be a blank before a function or class definition. Input: ```python import sys if sys.version_info > (3, 7): class Nested1: assignment = 1 def function_definition(self): ... def f1(self) -> str: ... class Nested2: def function_definition(self): ... assignment = 1 def f2(self) -> str: ... if sys.version_info > (3, 7): def nested1(): assignment = 1 def function_definition(self): ... def f1(self) -> str: ... def nested2(): def function_definition(self): ... assignment = 1 def f2(self) -> str: ... ``` Stable style ```python import sys if sys.version_info > (3, 7): class Nested1: assignment = 1 def function_definition(self): ... def f1(self) -> str: ... class Nested2: def function_definition(self): ... assignment = 1 def f2(self) -> str: ... if sys.version_info > (3, 7): def nested1(): assignment = 1 def function_definition(self): ... def f1(self) -> str: ... def nested2(): def function_definition(self): ... assignment = 1 def f2(self) -> str: ... ``` In the stable formatting, we have a blank line sometimes, not depending on the previous statement on the same level, but on the last (potentially nested) statement in the previous body. psf#2783/psf#3564 fixes this for classes in preview style: ```python import sys if sys.version_info > (3, 7): class Nested1: assignment = 1 def function_definition(self): ... def f1(self) -> str: ... class Nested2: def function_definition(self): ... assignment = 1 def f2(self) -> str: ... if sys.version_info > (3, 7): def nested1(): assignment = 1 def function_definition(self): ... def f1(self) -> str: ... def nested2(): def function_definition(self): ... assignment = 1 def f2(self) -> str: ... ``` This PR additionally fixes this for function definitions: ```python if sys.version_info > (3, 7): if sys.platform == "win32": assignment = 1 def function_definition(self): ... def f1(self) -> str: ... if sys.platform != "win32": def function_definition(self): ... assignment = 1 def f2(self) -> str: ... if sys.version_info > (3, 8): if sys.platform == "win32": assignment = 1 def function_definition(self): ... class F1: ... if sys.platform != "win32": def function_definition(self): ... assignment = 1 class F2: ... ``` You can see the effect of this change on typeshed in https://github.com/konstin/typeshed/pull/1/files. As baseline, the preview mode changes without this PR are at konstin/typeshed#2.

JelleZijlstra · 2023-09-08T14:54:06Z

Thanks for the PR and for running this on typeshed! I'd like to hear what other typeshed maintainers (@srittau, @Akuli, @AlexWaygood, @hauntsaninja) think of the changes in https://github.com/konstin/typeshed/pull/1/files. Personally I'm supportive; they do make the code more clear.

AlexWaygood · 2023-09-08T14:59:09Z

I'd like to hear what other typeshed maintainers (@srittau, @Akuli, @AlexWaygood, @hauntsaninja) think of the changes in https://github.com/konstin/typeshed/pull/1/files. Personally I'm supportive; they do make the code more clear.

Yeah, agreed, those pretty much all look like clear improvements to me! Thanks for working on this @konstin :D

github-actions · 2023-09-08T15:15:16Z

diff-shades results comparing this PR (870f12c) to main (a20338c). The full diff is available in the logs under the "Generate HTML diff report" step.

╭─────────────────────── Summary ────────────────────────╮
│ 1 projects & 13 files changed / 23 changes [+23/-0]    │
│                                                        │
│ ... out of 2 484 609 lines, 11 700 files & 23 projects │
╰────────────────────────────────────────────────────────╯

Differences found.

What is this? | Workflow run | diff-shades documentation

src/black/lines.py

srittau · 2023-09-08T15:57:52Z

LGTM

JelleZijlstra

Looks good from an implementation perspective, will wait a little longer for feedback on whether the formatting change is good.

## Summary Fix all but one empty line differences with the black preview style in typeshed. The remaining differences are breaking with type comments and trailing commas in function definitions. I compared the empty line differences with the preview mode of black since stable has some oddities that would have been hard to replicate (psf/black#3861). Additionally, it assumes the style proposed in psf/black#3862. An edge case that also surfaced with typeshed are newline before trailing module comments. **main** | project | similarity index | total files | changed files | |--------------|------------------:|------------------:|------------------:| | cpython | 0.76083 | 1789 | 1632 | | django | 0.99966 | 2760 | 58 | | transformers | 0.99930 | 2587 | 447 | | twine | 1.00000 | 33 | 0 | | **typeshed** | 0.99978 | 3496 | **2173** | | warehouse | 0.99825 | 648 | 22 | | zulip | 0.99950 | 1437 | 27 | **PR** | project | similarity index | total files | changed files | |--------------|------------------:|------------------:|------------------:| | cpython | 0.76083 | 1789 | 1632 | | django | 0.99966 | 2760 | 58 | | transformers | 0.99930 | 2587 | 447 | | twine | 1.00000 | 33 | 0 | | **typeshed** | 0.99983 | 3496 | **18** | | warehouse | 0.99825 | 648 | 22 | | zulip | 0.99950 | 1437 | 27 | Closes #6723 ## Test Plan The main driver was the typeshed diff. I added new test cases for all kinds of possible empty line combinations in stub files, test cases for newlines before trailing module comments. --------- Co-authored-by: Micha Reiser <micha@reiser.io>

konstin mentioned this pull request Sep 8, 2023

Format empty lines in stub files like black's preview style astral-sh/ruff#7206

Merged

JelleZijlstra reviewed Sep 8, 2023

View reviewed changes

src/black/lines.py Outdated Show resolved Hide resolved

Update src/black/lines.py

870f12c

JelleZijlstra approved these changes Sep 8, 2023

View reviewed changes

hauntsaninja closed this Sep 8, 2023

hauntsaninja reopened this Sep 8, 2023

hauntsaninja approved these changes Sep 8, 2023

View reviewed changes

JelleZijlstra merged commit b40b01f into psf:main Sep 9, 2023
35 checks passed

konstin deleted the blank_line_after_body_in_stub_file branch September 9, 2023 06:29

hauntsaninja mentioned this pull request Nov 13, 2023

Setting the 2024 stable style #4042

Closed

konstin mentioned this pull request Nov 14, 2023

🏖️ Black 2024 Preview Style astral-sh/ruff#8678

Closed

28 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Blank line between nested and function def in stub files. #3862

Blank line between nested and function def in stub files. #3862

konstin commented Sep 8, 2023 •

edited

JelleZijlstra commented Sep 8, 2023

AlexWaygood commented Sep 8, 2023

github-actions bot commented Sep 8, 2023 •

edited

srittau commented Sep 8, 2023

JelleZijlstra left a comment

Blank line between nested and function def in stub files. #3862

Blank line between nested and function def in stub files. #3862

Conversation

konstin commented Sep 8, 2023 • edited

Description

Checklist - did you ...

JelleZijlstra commented Sep 8, 2023

AlexWaygood commented Sep 8, 2023

github-actions bot commented Sep 8, 2023 • edited

srittau commented Sep 8, 2023

JelleZijlstra left a comment

Choose a reason for hiding this comment

konstin commented Sep 8, 2023 •

edited

github-actions bot commented Sep 8, 2023 •

edited