Skip to content

Commit

Permalink
Use more compatible ruff and isort options. (#1139)
Browse files Browse the repository at this point in the history
Currently, running `tox -e fmt` leaves the code in a state that `tox -e lint` complains about, for two reasons:

* Wrapping a line in ops/charm - I'm not sure why this difference exists since autopep8 and ruff have the same max line length (or why it wasn't picked up before we used ruff), but ruff is ok with the wrapped version, so it seems like the simple fix here is to just have it wrapped.
* "from x import a,b,c" style imports and wrapping.

For the import issue, if we adjust isort to use the "vertical hanging indent" mode (weirdly called "3") that gets most of the way there, but even though the docs show a trailing comma, one isn't included. However, if we use the `--split-on-trailing-comma` mode as well that does lead to a compatible format. We're not using the "force wrapping via trailing comma" trick anywhere, but it seems harmless enough to have it enabled.

I've put the options in tox.ini rather than pyproject.toml because we're intending to move to `ruff` for formatting soon (#1103) and this ensures that these options go away, rather than getting missed in pyproject.toml. I can change that if we'd rather keep all the options in one place.

Also bumped the isort version, which isn't required but I did while investigating, and since I've tested it and it's ok, seems reasonable. Also passed the line length to isort, which isn't needed at the moment but is more consistent.
  • Loading branch information
tonyandrewmeyer authored Feb 28, 2024
1 parent c623461 commit 9c0f44b
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 3 deletions.
4 changes: 3 additions & 1 deletion ops/charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -668,7 +668,9 @@ def restore(self, snapshot: Dict[str, Any]):

if storage_name and storage_index is not None:
storages = self.framework.model.storages[storage_name]
self.storage = next((s for s in storages if s.index == storage_index), None) # type: ignore
self.storage = next(
(s for s in storages if s.index == storage_index),
None) # type: ignore
if self.storage is None:
msg = 'failed loading storage (name={!r}, index={!r}) from snapshot' \
.format(storage_name, storage_index)
Expand Down
4 changes: 2 additions & 2 deletions tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,9 @@ commands =
description = Apply coding style standards to code
deps =
autopep8~=1.6
isort~=5.11
isort~=5.13
commands =
isort {[vars]all_path}
isort {[vars]all_path} --multi-line=3 --line-length=99 --split-on-trailing-comma
autopep8 --in-place {[vars]all_path}

[testenv:lint]
Expand Down

0 comments on commit 9c0f44b

Please # to comment.