Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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