Merge pull request 'style: adopt ruff at 88, replacing black/isort/flake8' (#5) from style/ruff into develop
Reviewed-on: https://git.zavage.net/Zavage-Software/app_skellington/pulls/5
This commit is contained in:
@@ -1,11 +0,0 @@
|
|||||||
[flake8]
|
|
||||||
max-line-length=120
|
|
||||||
ignore =
|
|
||||||
E121,
|
|
||||||
E123,
|
|
||||||
E126,
|
|
||||||
E226,
|
|
||||||
E24,
|
|
||||||
E704,
|
|
||||||
W605
|
|
||||||
exclude = ./tests
|
|
||||||
@@ -3,4 +3,3 @@ dist
|
|||||||
*.egg-info
|
*.egg-info
|
||||||
__pycache__
|
__pycache__
|
||||||
.idea
|
.idea
|
||||||
|
|
||||||
|
|||||||
+7
-20
@@ -1,6 +1,6 @@
|
|||||||
repos:
|
repos:
|
||||||
- repo: https://github.com/pre-commit/pre-commit-hooks
|
- repo: https://github.com/pre-commit/pre-commit-hooks
|
||||||
rev: v4.5.0
|
rev: v6.0.0
|
||||||
hooks:
|
hooks:
|
||||||
- id: check-yaml
|
- id: check-yaml
|
||||||
- id: check-added-large-files
|
- id: check-added-large-files
|
||||||
@@ -9,23 +9,10 @@ repos:
|
|||||||
- id: end-of-file-fixer
|
- id: end-of-file-fixer
|
||||||
- id: trailing-whitespace
|
- id: trailing-whitespace
|
||||||
|
|
||||||
# isort -- sorts imports
|
# ruff replaces black, isort and flake8. Keep rev equal to the ruff pin in pyproject.toml.
|
||||||
- repo: https://github.com/timothycrosley/isort
|
- repo: https://github.com/astral-sh/ruff-pre-commit
|
||||||
rev: 5.13.2
|
rev: v0.16.10
|
||||||
hooks:
|
hooks:
|
||||||
- id: isort
|
- id: ruff-check
|
||||||
args: ["--profile", "black", "--filter-files"]
|
args: [--fix]
|
||||||
|
- id: ruff-format
|
||||||
# Flake8
|
|
||||||
- repo: https://github.com/pycqa/flake8
|
|
||||||
rev: '7.0.0'
|
|
||||||
hooks:
|
|
||||||
- id: flake8
|
|
||||||
|
|
||||||
# Black
|
|
||||||
# Using this mirror lets us use mypyc-compiled black, which is about 2x faster
|
|
||||||
- repo: https://github.com/psf/black-pre-commit-mirror
|
|
||||||
rev: 24.2.0
|
|
||||||
hooks:
|
|
||||||
- id: black
|
|
||||||
language_version: python3.8
|
|
||||||
|
|||||||
+1
-2
@@ -2,7 +2,7 @@
|
|||||||
|
|
||||||
## [0.2.2] (2020-07-19)
|
## [0.2.2] (2020-07-19)
|
||||||
|
|
||||||
Second release is focused on cleanup, documentation.
|
Second release is focused on cleanup, documentation.
|
||||||
|
|
||||||
* flake8, black, isort - all warnings and errors fixed.
|
* flake8, black, isort - all warnings and errors fixed.
|
||||||
* Documentation improved.
|
* Documentation improved.
|
||||||
@@ -19,4 +19,3 @@ First release to PyPi.
|
|||||||
* Config through ConfigObj ini. Define spec file and input config.ini
|
* Config through ConfigObj ini. Define spec file and input config.ini
|
||||||
* Colored Logging
|
* Colored Logging
|
||||||
* Services can be defined and provided to classes
|
* Services can be defined and provided to classes
|
||||||
|
|
||||||
|
|||||||
@@ -9,22 +9,20 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co
|
|||||||
## Commands
|
## Commands
|
||||||
|
|
||||||
```bash
|
```bash
|
||||||
pip install -e '.[dev]' pytest # dev extras = black, isort, flake8, pre-commit; pytest is NOT in the extras
|
pip install -e '.[dev]' pytest # dev extras = ruff, pre-commit; pytest is NOT in the extras
|
||||||
pytest tests # full suite from repo root (or `pytest .` from inside tests/)
|
pytest tests # full suite from repo root (or `pytest .` from inside tests/)
|
||||||
pytest tests/cfg/test_cfg.py::TestConfig_e2e::test_uses_spec_as_defaults # single test
|
pytest tests/cfg/test_cfg.py::TestConfig_e2e::test_uses_spec_as_defaults # single test
|
||||||
pytest tests -k "<keyword>"
|
pytest tests -k "<keyword>"
|
||||||
|
|
||||||
black app_skellington
|
ruff check --fix # lint + import sorting (config in pyproject.toml)
|
||||||
isort --profile black app_skellington
|
ruff format # formatter, line length 88
|
||||||
flake8 app_skellington # .flake8: max-line-length 120, tests/ excluded
|
|
||||||
|
|
||||||
python -m build # sdist + wheel into dist/
|
python -m build # sdist + wheel into dist/
|
||||||
python -m setuptools_scm # print the version the build will get
|
python -m setuptools_scm # print the version the build will get
|
||||||
```
|
```
|
||||||
|
|
||||||
- **Versioning is tag-driven.** setuptools_scm uses `release-branch-semver` and derives the version from the latest `vX.Y.Z` git tag. It writes `app_skellington/_version.py` at build time. That file is tracked, but don't hand-edit it. `pip install -e .` also rewrites it, so `git restore` it rather than committing the churn. A release means tagging, then `python -m build`, then `twine upload dist/*`. The README covers the full publish steps.
|
- **Versioning is tag-driven.** setuptools_scm uses `release-branch-semver` and derives the version from the latest `vX.Y.Z` git tag. It writes `app_skellington/_version.py` at build time. That file is tracked, but don't hand-edit it. `pip install -e .` also rewrites it, so `git restore` it rather than committing the churn. A release means tagging, then `python -m build`, then `twine upload dist/*`. The README covers the full publish steps.
|
||||||
- **Formatter config is inconsistent.** `pyproject.toml` has no `[tool.black]`, so black runs at its default width of 88. Only flake8 allows 120. The code on `main` is formatted at 88. The pre-commit black hook pins `language_version: python3.8`. It fails unless a 3.8 interpreter is on PATH (`.python-version` names 3.8.x for pyenv).
|
- **ruff** handles formatting (88 columns) and lint (`E`, `F`, `W`, `I`). E501 is ignored on purpose, since the formatter owns line length. Keep the `ruff==` pin in `pyproject.toml` equal to the `rev:` in `.pre-commit-config.yaml`.
|
||||||
- **Lint is not currently clean on `main`.** black wants to reformat 2 files, isort flags 3, and flake8 reports W391 and W503. Keep unrelated reformatting out of feature diffs.
|
|
||||||
- Tests cover only `cfg` (ConfigObj fixtures live in `tests/cfg/`) plus a CommandTree constructor smoke test. `tests/test_log.py` is empty.
|
- Tests cover only `cfg` (ConfigObj fixtures live in `tests/cfg/`) plus a CommandTree constructor smoke test. `tests/test_log.py` is empty.
|
||||||
|
|
||||||
## Python support policy
|
## Python support policy
|
||||||
|
|||||||
@@ -4,13 +4,13 @@ import os
|
|||||||
|
|
||||||
import appdirs
|
import appdirs
|
||||||
|
|
||||||
from ._util import ServiceNotFound, NoCommandSpecified, get_root_asset
|
|
||||||
from . import log
|
from . import log
|
||||||
from .cfg import Config
|
|
||||||
from .cli import CommandTree
|
|
||||||
|
|
||||||
# Application scaffolding:
|
# Application scaffolding:
|
||||||
from ._bootstrap import _bootstrap_logger
|
from ._bootstrap import _bootstrap_logger
|
||||||
|
from ._util import NoCommandSpecified, ServiceNotFound, get_root_asset
|
||||||
|
from .cfg import Config
|
||||||
|
from .cli import CommandTree
|
||||||
|
|
||||||
# These two variables affect the directory paths for
|
# These two variables affect the directory paths for
|
||||||
# config files and logging.
|
# config files and logging.
|
||||||
@@ -189,5 +189,3 @@ class ApplicationContainer:
|
|||||||
def usage(self):
|
def usage(self):
|
||||||
pass
|
pass
|
||||||
# Applications need a default usage
|
# Applications need a default usage
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -217,9 +217,9 @@ class Config:
|
|||||||
try:
|
try:
|
||||||
_bootstrap_logger.info("cfg - Validating config file against spec")
|
_bootstrap_logger.info("cfg - Validating config file against spec")
|
||||||
val = validate.Validator()
|
val = validate.Validator()
|
||||||
assert isinstance(
|
assert isinstance(self._config_obj, configobj.ConfigObj), (
|
||||||
self._config_obj, configobj.ConfigObj
|
"expecting configobj.ConfigObj, received %s" % type(self._config_obj)
|
||||||
), "expecting configobj.ConfigObj, received %s" % type(self._config_obj)
|
)
|
||||||
# NOTE(MG) copy arg below instructs configobj to use defaults from spec file
|
# NOTE(MG) copy arg below instructs configobj to use defaults from spec file
|
||||||
test_results = self._config_obj.validate(
|
test_results = self._config_obj.validate(
|
||||||
val, copy=True, preserve_errors=True
|
val, copy=True, preserve_errors=True
|
||||||
|
|||||||
+10
-11
@@ -3,8 +3,8 @@ import inspect
|
|||||||
import re
|
import re
|
||||||
import sys
|
import sys
|
||||||
|
|
||||||
from ._util import NoCommandSpecified
|
|
||||||
from ._bootstrap import _bootstrap_logger
|
from ._bootstrap import _bootstrap_logger
|
||||||
|
from ._util import NoCommandSpecified
|
||||||
|
|
||||||
# If explicit fail is enabled, any command with at least one unknown
|
# If explicit fail is enabled, any command with at least one unknown
|
||||||
# argument will be rejected entirely. If not enabled, unknown arguments
|
# argument will be rejected entirely. If not enabled, unknown arguments
|
||||||
@@ -233,22 +233,21 @@ class CommandTree:
|
|||||||
# the CommandTree with no SubMenu (submenu will be disabled
|
# the CommandTree with no SubMenu (submenu will be disabled
|
||||||
# in this case):
|
# in this case):
|
||||||
if self._cmd_tree_is_single_command:
|
if self._cmd_tree_is_single_command:
|
||||||
assert (
|
assert self._cmd_tree_is_single_command is True, (
|
||||||
self._cmd_tree_is_single_command is True
|
"corrupt data structure in CommandMenu"
|
||||||
), "corrupt data structure in CommandMenu"
|
)
|
||||||
assert self._entries is None, "corrupt data structure in CommandMenu"
|
assert self._entries is None, "corrupt data structure in CommandMenu"
|
||||||
assert isinstance(
|
assert isinstance(self._single_command, CommandEntry), (
|
||||||
self._single_command, CommandEntry
|
"corrupt data structure in CommandMenu"
|
||||||
), "corrupt data structure in CommandMenu"
|
)
|
||||||
return self._single_command
|
return self._single_command
|
||||||
|
|
||||||
# There is at least one submenu we need to go down:
|
# There is at least one submenu we need to go down:
|
||||||
else:
|
else:
|
||||||
|
|
||||||
assert self._single_command is None, "corrupt data structure in CommandMenu"
|
assert self._single_command is None, "corrupt data structure in CommandMenu"
|
||||||
assert (
|
assert self._cmd_tree_is_single_command is False, (
|
||||||
self._cmd_tree_is_single_command is False
|
"corrupt data structure in CommandMenu"
|
||||||
), "corrupt data structure in CommandMenu"
|
)
|
||||||
|
|
||||||
# Key or variable name used by argparse to store the submenu options
|
# Key or variable name used by argparse to store the submenu options
|
||||||
argparse_param = self.submenu_param # e.g.: submenu_root
|
argparse_param = self.submenu_param # e.g.: submenu_root
|
||||||
|
|||||||
@@ -160,7 +160,9 @@ def norm(src):
|
|||||||
tree = ast.parse(src)
|
tree = ast.parse(src)
|
||||||
imports, rest = [], []
|
imports, rest = [], []
|
||||||
for node in tree.body:
|
for node in tree.body:
|
||||||
(imports if isinstance(node, (ast.Import, ast.ImportFrom)) else rest).append(node)
|
(imports if isinstance(node, (ast.Import, ast.ImportFrom)) else rest).append(
|
||||||
|
node
|
||||||
|
)
|
||||||
for node in imports:
|
for node in imports:
|
||||||
node.names = sorted(node.names, key=lambda a: (a.name, a.asname or ""))
|
node.names = sorted(node.names, key=lambda a: (a.name, a.asname or ""))
|
||||||
return sorted(ast.dump(n) for n in imports), [ast.dump(n) for n in rest]
|
return sorted(ast.dump(n) for n in imports), [ast.dump(n) for n in rest]
|
||||||
@@ -876,8 +878,7 @@ import pytest # add to the imports
|
|||||||
|
|
||||||
|
|
||||||
@pytest.mark.skipif(not VERIFY_SCRIPT.exists(), reason="added with the release task")
|
@pytest.mark.skipif(not VERIFY_SCRIPT.exists(), reason="added with the release task")
|
||||||
def test_verify_script_covers_matrix():
|
def test_verify_script_covers_matrix(): ...
|
||||||
...
|
|
||||||
```
|
```
|
||||||
Run again. Expected: all pass, 1 skipped.
|
Run again. Expected: all pass, 1 skipped.
|
||||||
|
|
||||||
|
|||||||
+16
-3
@@ -33,13 +33,26 @@ documentation = "https://git-repos.zavage.net/Zavage-Software/app_skellington"
|
|||||||
|
|
||||||
[project.optional-dependencies]
|
[project.optional-dependencies]
|
||||||
dev = [
|
dev = [
|
||||||
"black",
|
"ruff==0.16.10",
|
||||||
"pre-commit",
|
"pre-commit",
|
||||||
"isort",
|
|
||||||
"flake8"
|
|
||||||
]
|
]
|
||||||
|
|
||||||
[tool.setuptools_scm]
|
[tool.setuptools_scm]
|
||||||
version_file = "app_skellington/_version.py"
|
version_file = "app_skellington/_version.py"
|
||||||
version_scheme = "release-branch-semver"
|
version_scheme = "release-branch-semver"
|
||||||
local_scheme = "node-and-date"
|
local_scheme = "node-and-date"
|
||||||
|
|
||||||
|
[tool.ruff]
|
||||||
|
line-length = 88
|
||||||
|
target-version = "py38"
|
||||||
|
extend-exclude = ["app_skellington/_version.py"]
|
||||||
|
|
||||||
|
[tool.ruff.lint]
|
||||||
|
# E/W = pycodestyle, F = pyflakes, I = isort: the old flake8 + isort coverage.
|
||||||
|
select = ["E", "F", "W", "I"]
|
||||||
|
# The formatter owns line length. Long strings and comments it can't wrap are allowed.
|
||||||
|
ignore = ["E501"]
|
||||||
|
|
||||||
|
[tool.ruff.lint.per-file-ignores]
|
||||||
|
# Placeholder smoke test. The characterization-tests plan rewrites it.
|
||||||
|
"tests/test_cli.py" = ["F841", "E712"]
|
||||||
|
|||||||
@@ -1,3 +1,2 @@
|
|||||||
[metadata]
|
[metadata]
|
||||||
description-file = README.md
|
description-file = README.md
|
||||||
|
|
||||||
|
|||||||
@@ -15,4 +15,3 @@ Run test by keyword:
|
|||||||
Run test by directory:
|
Run test by directory:
|
||||||
|
|
||||||
pytest <dirname>
|
pytest <dirname>
|
||||||
|
|
||||||
|
|||||||
@@ -2,4 +2,3 @@ root_option = root_option_val
|
|||||||
|
|
||||||
[app]
|
[app]
|
||||||
sub_option = sub_option_val
|
sub_option = sub_option_val
|
||||||
|
|
||||||
|
|||||||
@@ -2,4 +2,3 @@ root_option = string(max=255, default='def_string')
|
|||||||
|
|
||||||
[app]
|
[app]
|
||||||
sub_option = string(max=255, default='def_sub')
|
sub_option = string(max=255, default='def_sub')
|
||||||
|
|
||||||
|
|||||||
@@ -2,4 +2,3 @@ root_option = root_option_val
|
|||||||
|
|
||||||
[app]
|
[app]
|
||||||
sub_option = sub_option_val
|
sub_option = sub_option_val
|
||||||
|
|
||||||
|
|||||||
@@ -3,4 +3,3 @@ int_option = integer(min=0, max=100)
|
|||||||
|
|
||||||
[app]
|
[app]
|
||||||
sub_option = string(max=255, default='def_sub')
|
sub_option = string(max=255, default='def_sub')
|
||||||
|
|
||||||
|
|||||||
@@ -1,2 +1 @@
|
|||||||
root_option = invalid(max=255, default='def_string')
|
root_option = invalid(max=255, default='def_string')
|
||||||
|
|
||||||
|
|||||||
+15
-15
@@ -32,18 +32,18 @@ def sample_invalid_configspec_filepath():
|
|||||||
class TestConfig_e2e:
|
class TestConfig_e2e:
|
||||||
def test_allows_reading_ini_and_no_spec(self, sample_configini_filepath):
|
def test_allows_reading_ini_and_no_spec(self, sample_configini_filepath):
|
||||||
cfg = Config(configini_filepath=sample_configini_filepath)
|
cfg = Config(configini_filepath=sample_configini_filepath)
|
||||||
assert (
|
assert cfg["root_option"] == "root_option_val", (
|
||||||
cfg["root_option"] == "root_option_val"
|
"expecting default from config.spec (didnt get)"
|
||||||
), "expecting default from config.spec (didnt get)"
|
)
|
||||||
assert (
|
assert cfg["app"]["sub_option"] == "sub_option_val", (
|
||||||
cfg["app"]["sub_option"] == "sub_option_val"
|
"expecting default for sub option"
|
||||||
), "expecting default for sub option"
|
)
|
||||||
|
|
||||||
def test_allows_reading_spec_and_no_ini(self, sample_configspec_filepath):
|
def test_allows_reading_spec_and_no_ini(self, sample_configspec_filepath):
|
||||||
cfg = Config(configspec_filepath=sample_configspec_filepath)
|
cfg = Config(configspec_filepath=sample_configspec_filepath)
|
||||||
assert (
|
assert cfg["root_option"] == "def_string", (
|
||||||
cfg["root_option"] == "def_string"
|
"expecting default from config.spec (didnt get)"
|
||||||
), "expecting default from config.spec (didnt get)"
|
)
|
||||||
|
|
||||||
# NOTE(MG) Changed the functionality to not do it this way.
|
# NOTE(MG) Changed the functionality to not do it this way.
|
||||||
# def test_constructor_fails_with_invalid_spec(
|
# def test_constructor_fails_with_invalid_spec(
|
||||||
@@ -71,9 +71,9 @@ class TestConfig_e2e:
|
|||||||
assert cfg["root_option"] == "newval"
|
assert cfg["root_option"] == "newval"
|
||||||
|
|
||||||
cfg["app"]["sub_option"] = "another_new_val"
|
cfg["app"]["sub_option"] = "another_new_val"
|
||||||
assert (
|
assert cfg["app"]["sub_option"] == "another_new_val", (
|
||||||
cfg["app"]["sub_option"] == "another_new_val"
|
"expecting default for sub option"
|
||||||
), "expecting default for sub option"
|
)
|
||||||
|
|
||||||
def test_can_set_option_without_config(self):
|
def test_can_set_option_without_config(self):
|
||||||
cfg = Config()
|
cfg = Config()
|
||||||
@@ -86,7 +86,7 @@ class TestConfig_e2e:
|
|||||||
|
|
||||||
def test_uses_spec_as_defaults(self, sample_configspec_filepath):
|
def test_uses_spec_as_defaults(self, sample_configspec_filepath):
|
||||||
cfg = Config(configspec_filepath=sample_configspec_filepath)
|
cfg = Config(configspec_filepath=sample_configspec_filepath)
|
||||||
assert (
|
assert cfg["root_option"] == "def_string", (
|
||||||
cfg["root_option"] == "def_string"
|
"expecting default from config.spec (didnt get)"
|
||||||
), "expecting default from config.spec (didnt get)"
|
)
|
||||||
assert cfg["app"]["sub_option"] == "def_sub", "expecting default for sub option"
|
assert cfg["app"]["sub_option"] == "def_sub", "expecting default for sub option"
|
||||||
|
|||||||
@@ -1,4 +1,3 @@
|
|||||||
[pytest]
|
[pytest]
|
||||||
filterwarnings =
|
filterwarnings =
|
||||||
ignore::DeprecationWarning
|
ignore::DeprecationWarning
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user