You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
cleanup: Improve style, formatting; use flake8, isort
This is a style cleanup, to increase code clarity and consistency,
supported by manual inspection, flake8, and isort. Most, but not
all, of these changes are to formatting, or to the way imports are
sorted and grouped (including removing unused imports).
This adds flake8, which had a configuration in setup.cfg and seems
to have previously been used, as well as isort. It configures them
for consistency with black and with the overall preexisting style.
This includes changing the line length to 88, which black uses. The
old pep8 configuration is removed, because pep8 was renamed to
pycodestyle, and the old pycodestyle is removed, because it doesn't
need to be run directly, since flake8 runs it and passes it an
appropriate configuration derived from its own. This commit sets up
flake8 and isort for local use, but it does not add them to CI.
Although flake8 detects many things black does not, they partially
overlap in their detection of overly long lines. The black rule is
more nuanced, rightly skipping over long strings in test data that
would be less clear if split across multiple lines. However, flake8
detects comments and docstrings that should be wrapped, so I didn't
turn off the flake8 rule. (Such long strings in tests are the only
cases where an inline "noqa:" suppression was needed.)
Besides tool-recommended changes, these other changes are included:
- Added the common virtualenv dirs venv and .venv to .gitignore.
- Fixed typos in comments. Made a few clarity/consistency tweaks.
- Moved a buried module docstring to top-of-file and expanded it.
- Turned a docstring that was meant as a comment into a comment.
- Made the use of initial and final newlines in and next to
docstrings consistent within files, when doing so also made it
more consistent with the overall style used in the project.
- Moved __all__ above other code (as PEP-8 recommends).
- Changed one __all__ from tuple to list. __all__ can be any
sequence of strings, but the other, more prominent __all__ was a
list (and using a list is a much more common practice).
- Replaced a few relative imports with absolute imports, since
the overall style in this codebase is to use absolute imports.
- Changed "import X.Y as Y" to "from X import Y", which was
sometimes also used nearby and which sometimes allowed imports to
be consolidated.
- When "import X.Y" appeared *in X*, changed to "from X import Y".
(Importing a submodule makes it accessible via its parent, so
this has a similar effect. The main relevant differences, in this
codebase, are to omit the unnecessary X.X bindings, and to make
it easier for editors/IDEs and less experienced human programmers
to understand what the code is doing.)
- In select cases where it seemed to noticeably improve clarity,
changed "import X.Y" (outside X) into "from X import Y" and
shortened the code using Y accordingly. (Mostly only when other
imports "from X" were present and being used similarly to Y.)
- Used raw string literals for all regular expressions.
- Removed "noqa" (and applied tool-recommended fixes) in cases
where "noqa" seems to have been added for expediency or to work
around a combination of limitations in the old flake8
configuration and old behavior of black (rather than to express a
preference that the code be kept exactly as it was).
Notable omissions:
- Although flake8 and isort are configured to ignore versioneer.py,
black must still be called with "--extend-exclude=versioneer.py".
Customizing black's configuration would require a pyproject.toml
file to be added, so that is deferred for now.
- To limit scope, this does not create any new CI jobs. (Such jobs
would also not pass yet, due to the next two omissions.)
- No changes are made to conftest.py, since it may make sense to
make deprecation-related non-style changes at the same time.
- In test_regression_143, the unused "validate" variables are kept,
because non-stylistic considerations should determine what
changes, if any, are to be made there.
0 commit comments