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 and placing groups in the PEP-8 recommended order). I've tried to include only Python code changes that I think can be considered improvements whether or not flake8 and isort continue to be used (though I recommend them). This fully retains black style, as well as the central role of black as the main tool for enforcing and checking style in the project.
This adds flake8, which had a configuration in setup.cfg (and had previously been used with Hound), 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 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 that 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 virtual environment directories 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, which I think would be better done separately from the changes here.
To limit scope, this does not create any new CI linting jobs. Such jobs would not pass yet, due to the next two omissions. Furthermore, even if flake8 and isort linting is desired, there are choices about how, and if, adding CI jobs for them should be done, which probably don't need to be considered in order to evaluate the changes in this PR.
No changes are made to conftest.py, since it may make sense to make deprecation-related non-style changes at the same time. This is to avoid an unnecessary conflict with #252.
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. I've opened #253 separately for that.
This is a style cleanup, to increase code clarity and consistency, supported by manual inspection,
flake8
, andisort
. Most, but not all, of these changes are to formatting, or to the way imports are sorted and grouped (including removing unused imports and placing groups in the PEP-8 recommended order). I've tried to include only Python code changes that I think can be considered improvements whether or notflake8
andisort
continue to be used (though I recommend them). This fully retainsblack
style, as well as the central role ofblack
as the main tool for enforcing and checking style in the project.This adds
flake8
, which had a configuration insetup.cfg
(and had previously been used with Hound), as well asisort
. It configures them for consistency withblack
and with the overall preexisting style. This includes changing the line length to 88, whichblack
uses. The oldpep8
configuration is removed, becausepep8
was renamed topycodestyle
, and the oldpycodestyle
is removed, because it doesn't need to be run directly, sinceflake8
runs it and passes it an appropriate configuration derived from its own. This sets upflake8
andisort
for local use, but it does not add them to CI.Although
flake8
detects many thingsblack
does not, they partially overlap in their detection of overly long lines. Theblack
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 thatflake8
rule. (Such long strings in tests are the only cases where an inlinenoqa:
suppression was needed.)Besides tool-recommended changes, these other changes are included:
venv
and.venv
to.gitignore
.__all__
above other code (as PEP-8 recommends).__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).import X.Y as Y
tofrom X import Y
, which was sometimes also used nearby and which sometimes allowed imports to be consolidated.import X.Y
appeared inX
, changed tofrom 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 unnecessaryX.X
bindings, and to make it easier for editors/IDEs and less experienced human programmers to understand what the code is doing.)import X.Y
(outsideX
) intofrom X import Y
and shortened the code usingY
accordingly. (Mostly only when other importsfrom X
were present and being used similarly toY
.)noqa
(and applied tool-recommended fixes) in cases wherenoqa
seems to have been added for expediency or to work around a combination of limitations in the oldflake8
configuration and old behavior ofblack
(rather than to express a preference that the code be kept exactly as it was).Notable omissions:
flake8
andisort
are configured to ignoreversioneer.py
,black
must still be called with--extend-exclude=versioneer.py
. Customizingblack
's configuration would require apyproject.toml
file to be added, which I think would be better done separately from the changes here.flake8
andisort
linting is desired, there are choices about how, and if, adding CI jobs for them should be done, which probably don't need to be considered in order to evaluate the changes in this PR.conftest.py
, since it may make sense to make deprecation-related non-style changes at the same time. This is to avoid an unnecessary conflict with #252.test_regression_143
, the unusedvalidate
variables are kept, because non-stylistic considerations should determine what changes, if any, are to be made there. I've opened #253 separately for that.