Use Ruff #566
Use Ruff #566
Conversation
| - flake8-debugger==4.1.2 | ||
| - flake8-logging-format==0.9.0 |
There was a problem hiding this comment.
I did not activate these two in ruff. We do not use logging in AWB and the flake8-debugger seems rather niche.
| @@ -28,6 +28,7 @@ def find_installed_packages(python_bin: str | None = None) -> dict[str, str]: | |||
| [python_bin, "-m", "pip", "list", "--format=json"], | |||
| encoding="utf-8", | |||
| capture_output=True, | |||
| check=True, | |||
There was a problem hiding this comment.
This is a change in behaviour. We had implicitly check=False and we were not checking the return code of the command. So if pip list failed, we would probably blow up below on line 34.
Now instead run throws exception on non-zero command exit. This exception will then be caught on line 204. This seems like a better behaviour. I still need to test this manually. (not sure it's worth adding a test for this)
| @@ -376,7 +376,7 @@ def _ssh_keygen(self): | |||
| "", | |||
| ] | |||
| if not fpath.exists(): | |||
| subprocess.run(keygen_cmd, capture_output=True) | |||
| subprocess.run(keygen_cmd, capture_output=True, check=True) | |||
There was a problem hiding this comment.
ditto, we were not checking that the command ran correctly. Now we at least fail loudly.
| @@ -1028,6 +1028,7 @@ def test(self, _=None): | |||
| process_result = subprocess.run( | |||
| ["verdi", "computer", "test", "--print-traceback", self.label.value], | |||
| capture_output=True, | |||
| check=False, | |||
There was a problem hiding this comment.
No functional change here, since we check the return code below.
| @@ -152,8 +152,8 @@ class :class:`aiidalab_widgets_base.structures.StructureManagerWidget`. | |||
|
|
|||
| def __init__( | |||
| self, | |||
| embedded: bool = True, | |||
| title: str = None, | |||
There was a problem hiding this comment.
This type was not correct (missing Optional). We're currently not doing any actual typechecking so it's better to just remove it, wrong types are worse than no types.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #566 +/- ##
=======================================
Coverage 87.07% 87.07%
=======================================
Files 27 27
Lines 4649 4649
=======================================
Hits 4048 4048
Misses 601 601
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Besides flake8 plugins that we already had, I've added ruff-specific rules (
RUF). Especially the one that detects unnecessary list comprehension is quite useful and made a couple of places more readable.I've disabled a couple of rules that would take more work to resolve all.