Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use Ruff #566

Merged
merged 6 commits into from
Feb 28, 2024
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
41 changes: 8 additions & 33 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,49 +12,24 @@ repos:
- id: trailing-whitespace
exclude: miscellaneous/structures

- repo: https://github.com/astral-sh/ruff-pre-commit
rev: v0.2.2
hooks:
- id: ruff-format
exclude: ^docs/.*
- id: ruff
args: [--fix, --exit-non-zero-on-fix, --show-fixes]

- repo: https://github.com/jumanjihouse/pre-commit-hook-yamlfmt
rev: 0.2.3
hooks:
- id: yamlfmt

- repo: https://github.com/psf/black
rev: 23.12.1
hooks:
- id: black
language_version: python3 # Should be a command that runs python3.6+

- repo: https://github.com/PyCQA/flake8
rev: 6.1.0
hooks:
- id: flake8
args: [--count, --show-source, --statistics]
additional_dependencies:
- flake8-bugbear==23.2.13
- flake8-builtins==2.1.0
- flake8-comprehensions==3.10.1
- flake8-debugger==4.1.2
- flake8-logging-format==0.9.0
Comment on lines -35 to -36
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not activate these two in ruff. We do not use logging in AWB and the flake8-debugger seems rather niche.

- pep8-naming==0.13.3
- pyflakes==3.1.0
- tryceratops==1.1.0

- repo: https://github.com/pycqa/isort
rev: 5.13.2
hooks:
- id: isort
args: [--profile, black, --filter-files]

- repo: https://github.com/sirosen/check-jsonschema
rev: 0.27.3
hooks:
- id: check-github-workflows

- repo: https://github.com/asottile/pyupgrade
rev: v3.15.0
hooks:
- id: pyupgrade
args: [--py39-plus]

- repo: https://github.com/kynan/nbstripout
rev: 0.6.1
hooks:
Expand Down
3 changes: 2 additions & 1 deletion aiidalab_widgets_base/bug_report.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

).stdout

return {package["name"]: package["version"] for package in json.loads(output)}
Expand Down Expand Up @@ -160,7 +161,7 @@ def install_create_github_issue_exception_handler(output, url, labels=None):
display(welcome_message, app_with_work_chain_selector, footer)

"""
global _ORIGINAL_EXCEPTION_HANDLER
global _ORIGINAL_EXCEPTION_HANDLER # noqa

if labels is None:
labels = []
Expand Down
5 changes: 3 additions & 2 deletions aiidalab_widgets_base/computational_resources.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto, we were not checking that the command ran correctly. Now we at least fail loudly.


def _can_login(self):
"""Check if it is possible to login into the remote host."""
Expand Down Expand Up @@ -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,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No functional change here, since we check the return code below.

)

if process_result.returncode == 0:
Expand Down Expand Up @@ -1602,7 +1603,7 @@ def __init__(
enable_detailed_setup=True,
):
if not any((enable_detailed_setup, enable_quick_setup)):
raise ValueError( # noqa
raise ValueError(
"At least one of `enable_quick_setup` and `enable_detailed_setup` should be True."
)

Expand Down
4 changes: 2 additions & 2 deletions aiidalab_widgets_base/databases.py
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,8 @@ class :class:`aiidalab_widgets_base.structures.StructureManagerWidget`.

def __init__(
self,
embedded: bool = True,
title: str = None,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

embedded=True,
title=None,
**kwargs,
) -> None:
try:
Expand Down
4 changes: 1 addition & 3 deletions aiidalab_widgets_base/elns.py
Original file line number Diff line number Diff line change
Expand Up @@ -249,9 +249,7 @@ def update_list_of_elns(self):
self.write_to_config(config)
default_eln = None

self.eln_instance.options = [("Setup new ELN", {})] + [
(k, v) for k, v in config.items()
]
self.eln_instance.options = [("Setup new ELN", {}), *list(config.items())]
if default_eln:
self.eln_instance.label = default_eln

Expand Down
4 changes: 1 addition & 3 deletions aiidalab_widgets_base/export.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,6 @@ def export_aiida_subgraph(self, change=None): # pylint: disable=unused-argument
document.body.appendChild(link);
link.click();
document.body.removeChild(link);
""".format(
payload=payload, filename=f"export_{self.process.id}.aiida"
)
""".format(payload=payload, filename=f"export_{self.process.id}.aiida")
unkcpz marked this conversation as resolved.
Show resolved Hide resolved
)
display(javas)
8 changes: 3 additions & 5 deletions aiidalab_widgets_base/misc.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def copy_to_clipboard(self, change=None): # pylint:disable=unused-argument
from IPython.display import Javascript, display

javas = Javascript(
"""
f"""
function copyStringToClipboard (str) {{
// Create new element
var el = document.createElement('textarea');
Expand All @@ -37,10 +37,8 @@ def copy_to_clipboard(self, change=None): # pylint:disable=unused-argument
// Remove temporary element
document.body.removeChild(el);
}}
copyStringToClipboard("{selection}");
""".format(
selection=self.value
)
copyStringToClipboard("{self.value}");
"""
) # For the moment works for Chrome, but doesn't work for Firefox.
if self.value: # If no value provided - do nothing.
display(javas)
Expand Down
8 changes: 4 additions & 4 deletions aiidalab_widgets_base/process.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,10 +156,10 @@ def __init__(self, process=None, **kwargs):
self.info = ipw.HTML()

self.flat_mapping = self.generate_flat_mapping(process=process) or {}
inputs_list = [(key, value) for key, value in self.flat_mapping.items()]
inputs_list = list(self.flat_mapping.items())

self._inputs = ipw.Dropdown(
options=[("Select input", "")] + inputs_list,
options=[("Select input", ""), *inputs_list],
description="Select input:",
style={"description_width": "initial"},
disabled=False,
Expand Down Expand Up @@ -230,7 +230,7 @@ def __init__(self, process=None, **kwargs):
else []
)
outputs = ipw.Dropdown(
options=[("Select output", "")] + outputs_list,
options=[("Select output", ""), *outputs_list],
label="Select output",
description="Select outputs:",
style={"description_width": "initial"},
Expand Down Expand Up @@ -285,7 +285,7 @@ def __init__(
)
)
self.output = ipw.HTML()
super().__init__(children=[self.output] + self.followers, **kwargs)
super().__init__(children=[self.output, *self.followers], **kwargs)
self.update()

def update(self):
Expand Down
30 changes: 16 additions & 14 deletions aiidalab_widgets_base/structures.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,11 @@ def __init__(
self._structure_importers(importers),
self.viewer,
ipw.HBox(
store_and_description
+ [self.structure_label, self.structure_description]
[
*store_and_description,
self.structure_label,
self.structure_description,
]
),
]

Expand All @@ -131,7 +134,7 @@ def __init__(
accordion.set_title(0, "Edit Structure")
children += [accordion]

super().__init__(children=children + [self.output], **kwargs)
super().__init__(children=[*children, self.output], **kwargs)

def _structure_importers(self, importers):
"""Preparing structure importers."""
Expand Down Expand Up @@ -217,7 +220,7 @@ def store_structure(self, _=None):
):
# Make a link between self.input_structure and self.structure_node
@engine.calcfunction
def user_modifications(source_structure):
def user_modifications(_source_structure):
return self.structure_node

structure_node = user_modifications(self.input_structure)
Expand Down Expand Up @@ -469,7 +472,7 @@ class StructureExamplesWidget(ipw.VBox):

def __init__(self, examples, title="", **kwargs):
self.title = title
self.on_structure_selection = lambda structure_ase, name: None
self.on_structure_selection = lambda _structure_ase, _name: None
self._select_structure = ipw.Dropdown(
options=self.get_example_structures(examples)
)
Expand All @@ -483,7 +486,7 @@ def get_example_structures(examples):
raise TypeError(
f"parameter examples should be of type list, {type(examples)} given"
)
return [("Select structure", False)] + examples
return [("Select structure", False), *examples]

def _on_select_structure(self, change=None):
"""When structure is selected."""
Expand Down Expand Up @@ -687,7 +690,7 @@ class SmilesWidget(ipw.VBox):

def __init__(self, title=""):
self.title = title
try: # noqa: TC101
try:
from rdkit import Chem # noqa: F401
from rdkit.Chem import AllChem # noqa: F401
except ImportError:
Expand Down Expand Up @@ -1065,13 +1068,11 @@ def _apply_cell_transformation(self, _=None, atoms=None):
try:
atoms = make_supercell(atoms, mat)
except Exception as e:
self._status_message.message = """
self._status_message.message = f"""
<div class="alert alert-info">
<strong>The transformation matrix is wrong! {}</strong>
<strong>The transformation matrix is wrong! {e}</strong>
</div>
""".format(
e
)
"""
return
# translate
atoms.translate(-atoms.cell.array.dot(translate))
Expand Down Expand Up @@ -1546,8 +1547,9 @@ def copy_sel(self, _=None, atoms=None, selection=None):
add_atoms.translate([1.0, 0, 0])
atoms += add_atoms

self.structure, self.input_selection = atoms, list(
range(last_atom, last_atom + len(selection))
self.structure, self.input_selection = (
atoms,
list(range(last_atom, last_atom + len(selection))),
)

@_register_structure
Expand Down
4 changes: 2 additions & 2 deletions aiidalab_widgets_base/utils/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ def get_ase_from_file(fname, file_format=None): # pylint: disable=redefined-bui

def find_ranges(iterable):
"""Yield range of consecutive numbers."""
for group in mit.consecutive_groups(iterable):
group = list(group)
for grp in mit.consecutive_groups(iterable):
group = list(grp)
if len(group) == 1:
yield group[0]
else:
Expand Down
Loading