Skip to content

Commit

Permalink
Add flake8-import-order (#49)
Browse files Browse the repository at this point in the history
  • Loading branch information
Joshua Cannon authored Feb 12, 2021
1 parent fd0177f commit ebea7d9
Show file tree
Hide file tree
Showing 15 changed files with 247 additions and 154 deletions.
46 changes: 34 additions & 12 deletions docs/Coding-Conventions.md
Original file line number Diff line number Diff line change
Expand Up @@ -712,17 +712,30 @@ import ministry
URL = "http://python.org"
```

### [O.1.3] ✔️ **DO** Group imports by standard library, third party, then first_party
### [O.1.3] ✔️ **DO** Group imports by standard library, third party, then first_party 💻

> 🐍 This rule stems from [PEP 8](https://www.python.org/dev/peps/pep-0008)
Additionally, you should put a blank line between each group of imports.
> 💻 This rule is enforced by error codes I201, I202
Additionally, you should put a single blank line between each group of imports.

```python
# Bad
# Bad - will produce I201
import os
import ministry
import my_app.utils
```

```python
# Bad - will produce I202
import os

import cheese_shop

import ministry

import my_app.utils
```

```python
Expand All @@ -734,14 +747,26 @@ import ministry
import my_app.utils
```

### [O.1.4] ✔️ **DO** Use absolute imports
### [O.1.4] ✔️ **DO** List imports in alphabetical order 💻

> 💻 This rule is enforced by error code I100
```python
# Bad
import pathlib
import os
from abc import ABC
```

### [O.1.5] ✔️ **DO** Use absolute imports

> 🐍 This rule stems from [PEP 8](https://www.python.org/dev/peps/pep-0008)
ℹ️ An exception can be made for `__init__.py` files republishing child module declarations

```python
# Bad
from . import sibling
from .sibling import rivalry
```

Expand All @@ -750,7 +775,7 @@ from .sibling import rivalry
from my_app.relationships.sibling import rivalry
```

### [O.1.5]**DO NOT** Use wildcard imports 💻
### [O.1.6]**DO NOT** Use wildcard imports 💻

> 🐍 This rule stems from [PEP 8](https://www.python.org/dev/peps/pep-0008)
Expand All @@ -773,7 +798,7 @@ from ministry import silly_walk
import ministry
```

### [O.1.6]**DO NOT** Rely on a module's imported names
### [O.1.7]**DO NOT** Rely on a module's imported names

> 🐍 This rule stems from [PEP 8](https://www.python.org/dev/peps/pep-0008)
Expand All @@ -784,14 +809,11 @@ import ministry

```python
# Bad
# cheese_shop.py - Imports module `brie`
import brie

# customer.py - Relying on the fact that `cheese_shop` imported module `brie`
# Assuming the module cheese_shop imported module `brie`, the following would be wrong:
import cheese_shop.brie
```

### [O.1.7]**DO NOT** Import definitions that are not used 💻
### [O.1.8]**DO NOT** Import definitions that are not used 💻

> 💻 This rule is enforced by error code F401
Expand All @@ -800,7 +822,7 @@ import cheese_shop.brie
import os # Assuming os is never used
```

### [O.1.8]**DO NOT** Change an imported object's case 💻
### [O.1.9]**DO NOT** Change an imported object's case 💻

> 💻 This rule is enforced by error codes N811, N812, N813, N814, N817
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from collections import defaultdict
import logging
import re
import pathlib
import re

from ni_python_styleguide._acknowledge_existing_errors import _lint_errors_parser

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import re
import logging
from collections import namedtuple
import logging
import re

LintError = namedtuple("LintError", ["file", "line", "column", "code", "explanation"])

Expand Down
31 changes: 26 additions & 5 deletions ni_python_styleguide/_cli.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import click
import contextlib
import flake8.main.application
from io import StringIO
import logging
import pathlib

import click
import flake8.main.application
import toml
from io import StringIO

from ni_python_styleguide import _acknowledge_existing_errors

Expand All @@ -25,6 +26,8 @@ def _read_pyproject_toml(ctx, param, value):
except (toml.TomlDecodeError, OSError) as e:
raise click.FileError(filename=value, hint=f"Error reading configuration file: {e}")

ctx.ensure_object(dict)
ctx.obj["PYPROJECT"] = pyproject_data
config = pyproject_data.get("tool", {}).get("ni-python-styleguide", {})

config.pop("quiet", None)
Expand All @@ -36,7 +39,23 @@ def _read_pyproject_toml(ctx, param, value):
return value


class AllowConfigGroup(click.Group):
def _get_application_import_names(pyproject):
"""Return the application package name the config."""
# Otherwise override with what was specified
app_name = (
pyproject.get("tool", {})
.get("ni-python-styleguide", {})
.get("application-import-names", "")
)

# Allow the poetry name as a fallback
if not app_name:
app_name = pyproject.get("tool", {}).get("poetry", {}).get("name", "").replace("-", "_")

return f"{app_name},tests"


class ConfigGroup(click.Group):
"""click.Group subclass which allows for a config option to load options from."""

def __init__(self, *args, **kwargs):
Expand All @@ -61,7 +80,7 @@ def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)


@click.group(cls=AllowConfigGroup)
@click.group(cls=ConfigGroup)
@click.option(
"-v",
"--verbose",
Expand Down Expand Up @@ -94,6 +113,7 @@ def main(ctx, verbose, quiet, config, exclude, extend_exclude):
ctx.ensure_object(dict)
ctx.obj["VERBOSITY"] = verbose - quiet
ctx.obj["EXCLUDE"] = ",".join(filter(bool, [exclude.strip(","), extend_exclude.strip(",")]))
ctx.obj["APP_IMPORT_NAMES"] = _get_application_import_names(ctx.obj.get("PYPROJECT", {}))


def _lint(obj, format, extend_ignore, file_or_dir):
Expand All @@ -108,6 +128,7 @@ def _lint(obj, format, extend_ignore, file_or_dir):
# [tool.black] setting (which makes sense if you think about it)
# So we need to give it one
f"--black-config={(pathlib.Path(__file__).parent / 'config.toml').resolve()}",
f"--application-import-names={obj['APP_IMPORT_NAMES']}",
*file_or_dir,
]
app.run(list(filter(bool, args)))
Expand Down
5 changes: 5 additions & 0 deletions ni_python_styleguide/config.ini
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,15 @@ ignore =
D213 # Multi-line docstring summary should start at the second line
D400 # First line should end with a period

# flake8-import-order
I101 # The names in your from import are in the wrong order. (Enforced by E401)

# Flake8 includes mccabe by default.
# We have yet to evaluate it, so ignore the errors for now
extend-ignore=C90

# flake8-docstrings
docstring-convention=all

# flake8-import-order
import-order-style=google
Loading

0 comments on commit ebea7d9

Please sign in to comment.