Skip to content

Conversation

ichard26
Copy link
Collaborator

@ichard26 ichard26 commented Sep 2, 2021

Description

One curveball is that we still want two preceding newlines before blocks
that are probably logically disconnected. In other words:

if condition:

    def foo():
        return "hi"
                         # <- aside: this is the goal of this commit
else:

    def foo():
        return "cya"
                         # <- the two newlines spacing here should stay
                         #    since this probably isn't related
with open("db.json", encoding="utf-8") as f:
    data = f.read()

Unfortunately that means we have to special case specific clause types
instead of just being able to just for a colon leaf. The hack used here
is to check whether we're adding preceding newlines for a standalone or
dependent clause. "Standalone" being a clause that doesn't need another
clause to be valid (eg. if) and vice versa.

Checklist - did you ...

  • Add a CHANGELOG entry if necessary?
  • Add / update tests if necessary?
  • Add new / update outdated documentation? -> probably n/a?

Fixes GH-647

I understand if this is way too hacky to be acceptable but I spent too much time to not at least give it a shot :)

One curveball is that we still want two preceding newlines before blocks
that are probably logically disconnected. In other words:

    if condition:

        def foo():
            return "hi"
                             # <- aside: this is the goal of this commit
    else:

        def foo():
            return "cya"
                             # <- the two newlines spacing here should stay
                             #    since this probably isn't related
    with open("db.json", encoding="utf-8") as f:
        data = f.read()

Unfortunately that means we have to special case specific clause types
instead of just being able to just for a colon leaf. The hack used here
is to check whether we're adding preceding newlines for a standalone or
dependent clause. "Standalone" being a clause that doesn't need another
clause to be valid (eg. if) and vice versa.
@ichard26 ichard26 added the F: empty lines Wasting vertical space efficiently. label Sep 2, 2021
Copy link
Collaborator

@JelleZijlstra JelleZijlstra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good with a couple of suggestions for wording

Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
@ichard26 ichard26 force-pushed the more-consistent-spacing branch from 34d89bd to fe9d927 Compare September 25, 2021 22:59
@ichard26
Copy link
Collaborator Author

ichard26 commented Dec 1, 2021

Diff-shades result:

--- a/attrs:src/attr/_compat.py
+++ b/attrs:src/attr/_compat.py
@@ -109,11 +109,10 @@
     def just_warn(*args, **kw):  # pragma: no cover
         """
         We only warn on Python 3 because we are not aware of any concrete
         consequences of not setting the cell on Python 2.
         """
-
 
 else:  # Python 3 and later.
     from collections.abc import Mapping, Sequence  # noqa
 
     def just_warn(*args, **kw):

--- a/attrs:src/attr/_make.py
+++ b/attrs:src/attr/_make.py
@@ -624,11 +624,10 @@
         ):
             BaseException.__setattr__(self, name, value)
             return
 
         raise FrozenInstanceError()
-
 
 else:
 
     def _frozen_setattrs(self, name, value):
         """
@@ -1624,11 +1623,10 @@
             getattr(cls.__setattr__, "__module__", None)
             == _frozen_setattrs.__module__
             and cls.__setattr__.__name__ == _frozen_setattrs.__name__
         )
 
-
 else:
 
     def _has_frozen_base_class(cls):
         """
         Check whether *cls* has a frozen ancestor by looking at its
@@ -1927,11 +1925,10 @@
         lines.append("    working_set.remove(id(self))")
 
         return _make_method(
             "__repr__", "\n".join(lines), unique_filename, globs=globs
         )
-
 
 else:
 
     def _make_repr(attrs, ns, _):
         """

--- a/django:django/core/files/locks.py
+++ b/django:django/core/files/locks.py
@@ -90,11 +90,10 @@
         hfile = msvcrt.get_osfhandle(_fd(f))
         overlapped = OVERLAPPED()
         ret = UnlockFileEx(hfile, 0, 0, 0xFFFF0000, byref(overlapped))
         return bool(ret)
 
-
 else:
     try:
         import fcntl
 
         LOCK_SH = fcntl.LOCK_SH  # shared lock

--- a/django:tests/dispatch/tests.py
+++ b/django:tests/dispatch/tests.py
@@ -11,11 +11,10 @@
 
     def garbage_collect():
         # Collecting weakreferences can take two collections on PyPy.
         gc.collect()
         gc.collect()
-
 
 else:
 
     def garbage_collect():
         gc.collect()

--- a/django:tests/gis_tests/gis_migrations/migrations/0001_setup_extensions.py
+++ b/django:tests/gis_tests/gis_migrations/migrations/0001_setup_extensions.py
@@ -12,10 +12,9 @@
                 CreateExtension('postgis_raster'),
             ]
         else:
             operations = []
 
-
 else:
 
     class Migration(migrations.Migration):
         operations = []

--- a/django:tests/gis_tests/rasterapp/migrations/0001_setup_extensions.py
+++ b/django:tests/gis_tests/rasterapp/migrations/0001_setup_extensions.py
@@ -12,10 +12,9 @@
                 CreateExtension('postgis_raster'),
             ]
         else:
             operations = []
 
-
 else:
 
     class Migration(migrations.Migration):
         operations = []

--- a/django:tests/model_forms/models.py
+++ b/django:tests/model_forms/models.py
@@ -216,11 +216,10 @@
         image = models.ImageField(storage=temp_storage, upload_to=upload_to)
 
         def __str__(self):
             return self.description
 
-
 except ImportError:
     test_images = False
 
 
 class Homepage(models.Model):

--- a/django:tests/utils_tests/test_module_loading.py
+++ b/django:tests/utils_tests/test_module_loading.py
@@ -212,11 +212,10 @@
             self.importer = zipimporter(*args, **kwargs)
 
         def find_spec(self, path, target=None):
             return self.importer.find_spec(path, target)
 
-
 else:
 
     class TestFinder:
         def __init__(self, *args, **kwargs):
             self.importer = zipimporter(*args, **kwargs)

--- a/hypothesis:hypothesis-python/src/hypothesis/entry_points.py
+++ b/hypothesis:hypothesis-python/src/hypothesis/entry_points.py
@@ -36,11 +36,10 @@
             # so we'll retain this fallback logic for some time to come.  See also
             # https://importlib-metadata.readthedocs.io/en/latest/using.html
             eps = importlib_metadata.entry_points().get("hypothesis", [])
         yield from eps
 
-
 except ImportError:
     # But if we're not on Python >= 3.8 and the importlib_metadata backport
     # is not installed, we fall back to pkg_resources anyway.
     try:
         import pkg_resources

--- a/hypothesis:hypothesis-python/src/hypothesis/extra/cli.py
+++ b/hypothesis:hypothesis-python/src/hypothesis/extra/cli.py
@@ -67,11 +67,10 @@
 
     def main():
         """If `click` is not installed, tell the user to install it then exit."""
         sys.stderr.write(MESSAGE.format("click"))
         sys.exit(1)
-
 
 else:
     # Ensure that Python scripts in the current working directory are importable,
     # on the principle that Ghostwriter should 'just work' for novice users.  Note
     # that we append rather than prepend to the module search path, so this will

--- a/hypothesis:hypothesis-python/src/hypothesis/internal/coverage.py
+++ b/hypothesis:hypothesis-python/src/hypothesis/internal/coverage.py
@@ -102,11 +102,10 @@
             with check_block(f.__name__, 2):
                 return f(*args, **kwargs)
 
         return accept
 
-
 else:  # pragma: no cover
 
     def check_function(f: Func) -> Func:
         return f
 

--- a/hypothesis:hypothesis-python/tests/cover/test_pretty.py
+++ b/hypothesis:hypothesis-python/tests/cover/test_pretty.py
@@ -304,11 +304,10 @@
         assert "SA" in output
 
         sb = SB()
         output = pretty.pretty(super(SA, sb))
         assert "SA" in output
-
 
 except AttributeError:
 
     def test_super_repr():
         pretty.pretty(super(SA))

--- a/pillow:Tests/helper.py
+++ b/pillow:Tests/helper.py
@@ -27,11 +27,10 @@
     class test_image_results:
         @staticmethod
         def upload(a, b):
             a.show()
             b.show()
-
 
 elif "GITHUB_ACTIONS" in os.environ:
     HAS_UPLOADER = True
 
     class test_image_results:
@@ -41,11 +40,10 @@
             os.makedirs(dir_errors, exist_ok=True)
             tmpdir = tempfile.mkdtemp(dir=dir_errors)
             a.save(os.path.join(tmpdir, "a.png"))
             b.save(os.path.join(tmpdir, "b.png"))
             return tmpdir
-
 
 else:
     try:
         import test_image_results
 

--- a/pillow:src/PIL/Image.py
+++ b/pillow:src/PIL/Image.py
@@ -73,11 +73,10 @@
                     DeprecationWarning,
                     stacklevel=2,
                 )
                 return categories[name]
         raise AttributeError(f"module '{__name__}' has no attribute '{name}'")
-
 
 else:
 
     from . import PILLOW_VERSION
 

--- a/pillow:src/PIL/__init__.py
+++ b/pillow:src/PIL/__init__.py
@@ -38,11 +38,10 @@
     def __getattr__(name):
         if name == "PILLOW_VERSION":
             _raise_version_warning()
             return __version__
         raise AttributeError(f"module '{__name__}' has no attribute '{name}'")
-
 
 else:
 
     class _Deprecated_Version(str):
         def __str__(self):

--- a/pytest:src/_pytest/_argcomplete.py
+++ b/pytest:src/_pytest/_argcomplete.py
@@ -106,11 +106,10 @@
     filescompleter: Optional[FastFilesCompleter] = FastFilesCompleter()
 
     def try_argcomplete(parser: argparse.ArgumentParser) -> None:
         argcomplete.autocomplete(parser, always_complete_options=False)
 
-
 else:
 
     def try_argcomplete(parser: argparse.ArgumentParser) -> None:
         pass
 

--- a/pytest:src/_pytest/assertion/rewrite.py
+++ b/pytest:src/_pytest/assertion/rewrite.py
@@ -321,11 +321,10 @@
             # we ignore any failure to write the cache file
             # there are many reasons, permission-denied, pycache dir being a
             # file etc.
             return False
         return True
-
 
 else:
 
     def _write_pyc(
         state: "AssertionState",

--- a/pytest:src/_pytest/compat.py
+++ b/pytest:src/_pytest/compat.py
@@ -191,11 +191,10 @@
 
     @contextmanager
     def nullcontext():
         yield
 
-
 else:
     from contextlib import nullcontext as nullcontext  # noqa: F401
 
 
 def get_default_arg_names(function: Callable[..., Any]) -> Tuple[str, ...]:

--- a/pytest:src/_pytest/pathlib.py
+++ b/pytest:src/_pytest/pathlib.py
@@ -560,11 +560,10 @@
 if sys.platform.startswith("win"):
 
     def _is_same(f1: str, f2: str) -> bool:
         return Path(f1) == Path(f2) or os.path.samefile(f1, f2)
 
-
 else:
 
     def _is_same(f1: str, f2: str) -> bool:
         return os.path.samefile(f1, f2)
 

--- a/sqlalchemy:lib/sqlalchemy/engine/row.py
+++ b/sqlalchemy:lib/sqlalchemy/engine/row.py
@@ -27,11 +27,10 @@
     # The extra function embedding is needed so that the
     # reconstructor function has the same signature whether or not
     # the extension is present.
     def rowproxy_reconstructor(cls, state):
         return safe_rowproxy_reconstructor(cls, state)
-
 
 except ImportError:
 
     def rowproxy_reconstructor(cls, state):
         obj = cls.__new__(cls)

--- a/sqlalchemy:lib/sqlalchemy/processors.py
+++ b/sqlalchemy:lib/sqlalchemy/processors.py
@@ -168,8 +168,7 @@
         # For example, the Python implementation might return
         # Decimal('5.00000') whereas the C implementation will
         # return Decimal('5'). These are equivalent of course.
         return DecimalResultProcessor(target_class, "%%.%df" % scale).process
 
-
 except ImportError:
     globals().update(py_fallback())

--- a/sqlalchemy:lib/sqlalchemy/testing/util.py
+++ b/sqlalchemy:lib/sqlalchemy/testing/util.py
@@ -70,11 +70,10 @@
     def random_choices(population, k=1):
         pop = list(population)
         # lame but works :)
         random.shuffle(pop)
         return pop[0:k]
-
 
 else:
 
     def random_choices(population, k=1):
         return random.choices(population, k=k)

--- a/sqlalchemy:lib/sqlalchemy/util/compat.py
+++ b/sqlalchemy:lib/sqlalchemy/util/compat.py
@@ -224,11 +224,10 @@
 
     from abc import ABC
 
     def _qualname(fn):
         return fn.__qualname__
-
 
 else:
     import base64
     import ConfigParser as configparser  # noqa
     import itertools
@@ -429,11 +428,10 @@
         result = "(" + ", ".join(specs) + ")"
         if "return" in annotations:
             result += formatreturns(formatannotation(annotations["return"]))
         return result
 
-
 else:
     from inspect import formatargspec as _inspect_formatargspec
 
     def inspect_formatargspec(*spec, **kw):
         # convert for a potential FullArgSpec from compat.getfullargspec()
@@ -471,11 +469,10 @@
             return [
                 f for f in dataclasses.fields(cls) if f not in super_fields
             ]
         else:
             return []
-
 
 else:
 
     def dataclass_fields(cls):
         return []

--- a/virtualenv:src/virtualenv/util/path/_sync.py
+++ b/virtualenv:src/virtualenv/util/path/_sync.py
@@ -12,11 +12,10 @@
 
 if PY2 and IS_CPYTHON and IS_WIN:  # CPython2 on Windows supports unicode paths if passed as unicode
 
     def norm(src):
         return ensure_text(str(src))
-
 
 else:
     norm = str
 
 

--- a/virtualenv:tasks/__main__zipapp.py
+++ b/virtualenv:tasks/__main__zipapp.py
@@ -128,11 +128,10 @@
                 return spec
 
         def module_repr(self, module):
             raise NotImplementedError
 
-
 else:
     # noinspection PyDeprecation
     from imp import new_module
 
     class VersionedFindLoad(VersionPlatformSelect):

@ichard26 ichard26 merged commit b0c2bcc into main Dec 1, 2021
@ichard26 ichard26 deleted the more-consistent-spacing branch December 1, 2021 23:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F: empty lines Wasting vertical space efficiently.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inconsistent spacing around top level functions
3 participants