Work on pre-commit hooks, small readme update. (#401)

Changes to pre-commit setup and to the `README.md` * Remove unused .prospector.yaml * Readme: add --tag-num option to update tag number * Flake8: add more plugins to better analyse the code: * `flake8-bugbear` for detecting bugs and potential design problems * `flake8-builtins` for verifying that builtins are not used as variable names * `flake8-comprehensions` for writing better and consistent comprehensions * `flake8-debugger` for checking that there are no forgotten breakpoints * `flake8-logging-format` for consistent logging * pep8-naming `for verifying that PEP8 naming conventions are followed` * `pyflakes` for checking source files for various errors * `ryceratops` to avoid exception anti-patterns * Add nbstripout hook to strip output from Jupyter and IPython notebooks * Changes requested by the pre-commit hooks: * Apply suggestions from flake8 * Fix an exception type: ValueError->TypeError * Fix TC003 * Fix TC101 errors Co-authored-by: Daniel Hollas <[email protected]> Co-authored-by: Jusong Yu <[email protected]>
aiidalab · Dec 12, 2022 · d48415a · d48415a
1 parent 1a8a8cf
commit d48415a
Show file tree

Hide file tree

Showing 16 changed files with 113 additions and 117 deletions.
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -25,6 +25,8 @@ repos:
       hooks:
           - id: flake8
             args: [--count, --show-source, --statistics]
+            additional_dependencies: [flake8-bugbear, flake8-builtins, flake8-comprehensions, flake8-debugger, flake8-logging-format, pep8-naming, pyflakes,
+                tryceratops]
 
     - repo: https://github.com/pycqa/isort
       rev: 5.10.1
@@ -47,3 +49,8 @@ repos:
       hooks:
           - id: pyupgrade
             args: [--py37-plus]
+
+    - repo: https://github.com/kynan/nbstripout
+      rev: 0.6.0
+      hooks:
+          - id: nbstripout
diff --git a/.prospector.yaml b/.prospector.yaml
diff --git a/README.md b/README.md
@@ -17,7 +17,7 @@ Hosted on [aiidalab-widgets-base.readthedocs.io](https://aiidalab-widgets-base.r
 
 ## For maintainers
 
-To create a new release, clone the repository, install development dependencies with `pip install -e '.[dev]'`, and then execute `bumpver update [--major|--minor|--patch] [--tag [alpha|beta|rc]]`.
+To create a new release, clone the repository, install development dependencies with `pip install -e '.[dev]'`, and then execute `bumpver update [--major|--minor|--patch|--tag-num] [--tag [alpha|beta|rc]]`.
 This will:
 
   1. Create a tagged release with bumped version and push it to the repository.

diff --git a/aiidalab_widgets_base/bug_report.py b/aiidalab_widgets_base/bug_report.py
@@ -111,11 +111,11 @@ def _convert_ansi_codes_to_html(msg):
 def _format_truncated_traceback(traceback, max_num_chars=2000):
     """Truncate the traceback to the given character length."""
     n = 0
-    for i, line in enumerate(reversed(traceback)):
+    for _i, line in enumerate(reversed(traceback)):
         n += len(_strip_ansi_codes(line)) + 2  # add 2 for newline control characters
         if n > max_num_chars:
             break
-    return _strip_ansi_codes("\n".join(traceback[-i:]))
+    return _strip_ansi_codes("\n".join(traceback[-_i:]))
 
 
 _ORIGINAL_EXCEPTION_HANDLER = None

diff --git a/aiidalab_widgets_base/computational_resources.py b/aiidalab_widgets_base/computational_resources.py
@@ -835,14 +835,13 @@ def _configure_computer(self, computer: orm.Computer):
             "use_login_shell": self.use_login_shell.value,
             "safe_interval": self.safe_interval.value,
         }
-        if "user" in sshcfg:
+        try:
             authparams["username"] = sshcfg["user"]
-        else:
-            print(
-                f"SSH username is not provided, please run `verdi computer configure {self.label.value}` "
-                "from the command line."
-            )
-            return False
+        except KeyError as exc:
+            message = "SSH username is not provided"
+            self.message = message
+            raise RuntimeError(message) from exc
+
         if "proxycommand" in sshcfg:
             authparams["proxy_command"] = sshcfg["proxycommand"]
         elif "proxyjump" in sshcfg:
@@ -854,19 +853,27 @@ def _configure_computer(self, computer: orm.Computer):
 
         return True
 
+    def _run_callbacks_if_computer_exists(self, label):
+        """Run things on an existing computer"""
+        try:
+            orm.Computer.objects.get(label=label)
+            for function in self._on_setup_computer_success:
+                function()
+        except common.NotExistent:
+            return False
+        else:
+            return True
+
     def on_setup_computer(self, _=None):
         """Create a new computer."""
         if self.label.value == "":  # check hostname
             self.message = "Please specify the computer name (for AiiDA)"
             return False
-        try:
-            computer = orm.Computer.objects.get(label=self.label.value)
+
+        # If the computer already exists, we just run the registered functions and return
+        if self._run_callbacks_if_computer_exists(self.label.value):
             self.message = f"A computer called {self.label.value} already exists."
-            for function in self._on_setup_computer_success:
-                function()
             return True
-        except common.NotExistent:
-            pass
 
         items_to_configure = [
             "label",
@@ -890,24 +897,24 @@ def on_setup_computer(self, _=None):
         )
         try:
             computer = computer_builder.new()
+            self._configure_computer(computer)
         except (
             ComputerBuilder.ComputerValidationError,
             common.exceptions.ValidationError,
+            RuntimeError,
         ) as err:
-            self.message = f"{type(err).__name__}: {err}"
+            self.message = f"Failed to setup computer {type(err).__name__}: {err}"
             return False
-
-        try:
+        else:
             computer.store()
-        except common.exceptions.ValidationError as err:
-            self.message = f"Unable to store the computer: {err}."
-            return False
 
-        if self._configure_computer(computer):
-            for function in self._on_setup_computer_success:
-                function()
-        self.message = f"Computer<{computer.pk}> {computer.label} created"
-        return True
+        # Callbacks will not run if the computer is not stored
+        if self._run_callbacks_if_computer_exists(self.label.value):
+            self.message = f"Computer<{computer.pk}> {computer.label} created"
+            return True
+
+        self.message = f"Failed to create computer {computer.label}"
+        return False
 
     def on_setup_computer_success(self, function):
         self._on_setup_computer_success.append(function)

diff --git a/aiidalab_widgets_base/databases.py b/aiidalab_widgets_base/databases.py
@@ -98,7 +98,7 @@ def _on_click_query(self, change):  # pylint: disable=unused-argument
             try:
                 entry_cif = entry.get_cif_node()
                 formula = entry_cif.get_ase().get_chemical_formula()
-            except:  # noqa: E722
+            except Exception:
                 continue
             entry_add = (
                 "{} (id: {})".format(formula, entry.source["id"]),

diff --git a/aiidalab_widgets_base/misc.py b/aiidalab_widgets_base/misc.py
@@ -138,9 +138,10 @@ def is_number(string):
             """Check if string is a number."""
             try:
                 float(string)
-                return True
             except ValueError:
                 return False
+            else:
+                return True
 
         stack = []
         stackposition = -1

diff --git a/aiidalab_widgets_base/nodes.py b/aiidalab_widgets_base/nodes.py
@@ -73,7 +73,7 @@
 class AiidaNodeTreeNode(TreeNode):
     def __init__(self, pk, name, **kwargs):
         self.pk = pk
-        self.nodes_registry = dict()
+        self.nodes_registry = {}
         super().__init__(name=name, **kwargs)
 
     @traitlets.default("opened")
@@ -103,11 +103,9 @@ class AiidaOutputsTreeNode(TreeNode):
     icon = traitlets.Unicode("folder").tag(sync=True)
     disabled = traitlets.Bool(True).tag(sync=True)
 
-    def __init__(
-        self, name, parent_pk, namespaces: Tuple[str, ...] = tuple(), **kwargs
-    ):
+    def __init__(self, name, parent_pk, namespaces: Tuple[str, ...] = (), **kwargs):
         self.parent_pk = parent_pk
-        self.nodes_registry = dict()
+        self.nodes_registry = {}
         self.namespaces = namespaces
         super().__init__(name=name, **kwargs)
 
@@ -173,13 +171,11 @@ def _convert_to_tree_nodes(self, old_nodes, new_nodes):
 
     @traitlets.observe("nodes")
     def _observe_nodes(self, change):
-        self._tree.nodes = list(
-            sorted(
-                self._convert_to_tree_nodes(
-                    old_nodes=self._tree.nodes, new_nodes=change["new"]
-                ),
-                key=lambda node: node.pk,
-            )
+        self._tree.nodes = sorted(
+            self._convert_to_tree_nodes(
+                old_nodes=self._tree.nodes, new_nodes=change["new"]
+            ),
+            key=lambda node: node.pk,
         )
         self.update()
         self._refresh_output()

diff --git a/aiidalab_widgets_base/process.py b/aiidalab_widgets_base/process.py
@@ -36,6 +36,7 @@
 from traitlets import Instance, Int, List, Unicode, default, observe, validate
 
 from .nodes import NodesTreeWidget
+from .utils import exceptions
 
 # Local imports.
 from .viewers import viewer
@@ -94,9 +95,9 @@ def __init__(  # pylint: disable=too-many-arguments
         if callable(inputs_generator):
             self.inputs_generator = inputs_generator
         else:
-            raise ValueError(
+            raise TypeError(
                 "The `inputs_generator` argument must be a function that "
-                f"returns input dictionary, got {inputs_generator}"
+                f"returns input dictionary, got {type(inputs_generator)}"
             )
 
         self.disable_after_submit = disable_after_submit
@@ -289,9 +290,7 @@ def follow(self, detach=False):
     def on_completed(self, function):
         """Run functions after a process has been completed."""
         if self._monitor is not None:
-            raise RuntimeError(
-                "Can not register new on_completed callback functions after following has already been initiated."
-            )
+            raise exceptions.CantRegisterCallbackError(function)
         self._run_after_completed.append(function)
 
 

diff --git a/aiidalab_widgets_base/structures.py b/aiidalab_widgets_base/structures.py
@@ -33,7 +33,7 @@
 
 # Local imports
 from .data import LigandSelectorWidget
-from .utils import StatusHTML, get_ase_from_file, get_formula
+from .utils import StatusHTML, exceptions, get_ase_from_file, get_formula
 from .viewers import StructureDataViewer
 
 CifData = DataFactory("core.cif")
@@ -149,11 +149,8 @@ def __init__(
 
     def _structure_importers(self, importers):
         """Preparing structure importers."""
-        if not importers:
-            raise ValueError(
-                "The parameter importers should contain a list (or tuple) of "
-                "importers, got a falsy object."
-            )
+        if not isinstance(importers, (list, tuple)):
+            raise exceptions.ListOrTuppleError(importers)
 
         # If there is only one importer - no need to make tabs.
         if len(importers) == 1:
@@ -163,7 +160,7 @@ def _structure_importers(self, importers):
 
         # Otherwise making one tab per importer.
         importers_tab = ipw.Tab()
-        importers_tab.children = [i for i in importers]  # One importer per tab.
+        importers_tab.children = list(importers)  # One importer per tab.
         for i, importer in enumerate(importers):
             # Labeling tabs.
             importers_tab.set_title(i, importer.title)
@@ -480,10 +477,8 @@ def __init__(self, examples, title="", **kwargs):
     def get_example_structures(examples):
         """Get the list of example structures."""
         if not isinstance(examples, list):
-            raise ValueError(
-                "parameter examples should be of type list, {} given".format(
-                    type(examples)
-                )
+            raise TypeError(
+                f"parameter examples should be of type list, {type(examples)} given"
             )
         return [("Select structure", False)] + examples
 
@@ -693,7 +688,7 @@ def __init__(self, title=""):
         except ImportError:
             self.disable_openbabel = True
 
-        try:
+        try:  # noqa: TC101
             from rdkit import Chem  # noqa: F401
             from rdkit.Chem import AllChem  # noqa: F401
         except ImportError:
@@ -1210,13 +1205,13 @@ def def_point(self, _=None):
         """Define the action point."""
         self.point.value = self.vec2str(self.sel2com())
         if self.autoclear_selection.value:
-            self.selection = list()
+            self.selection = []
 
     def def_axis_p1(self, _=None):
         """Define the first point of axis."""
         self.axis_p1.value = self.vec2str(self.sel2com())
         if self.autoclear_selection.value:
-            self.selection = list()
+            self.selection = []
 
     def def_axis_p2(self, _=None):
         """Define the second point of axis."""
@@ -1234,7 +1229,7 @@ def def_axis_p2(self, _=None):
             )
             self.axis_p2.value = self.vec2str(com)
             if self.autoclear_selection.value:
-                self.selection = list()
+                self.selection = []
 
     def def_perpendicular_to_screen(self, _=None):
         """Define a normalized vector perpendicular to the screen."""
@@ -1373,9 +1368,9 @@ def mod_element(self, _=None, atoms=None, selection=None):
                 lgnd = initial_ligand.copy()
                 lgnd.translate(position)
                 atoms += lgnd
-            new_selection = [
-                i for i in range(last_atom, last_atom + len(selection) * len(lgnd))
-            ]
+            new_selection = list(
+                range(last_atom, last_atom + len(selection) * len(lgnd))
+            )
 
         self.structure, self.selection = atoms, new_selection
 
@@ -1390,7 +1385,7 @@ def copy_sel(self, _=None, atoms=None, selection=None):
         add_atoms.translate([1.0, 0, 0])
         atoms += add_atoms
 
-        new_selection = [i for i in range(last_atom, last_atom + len(selection))]
+        new_selection = list(range(last_atom, last_atom + len(selection)))
 
         self.structure, self.selection = atoms, new_selection
 
@@ -1423,9 +1418,7 @@ def add(self, _=None, atoms=None, selection=None):
 
             atoms += lgnd
 
-        new_selection = [
-            i for i in range(last_atom, last_atom + len(selection) * len(lgnd))
-        ]
+        new_selection = list(range(last_atom, last_atom + len(selection) * len(lgnd)))
 
         self.structure, self.selection = atoms, new_selection
 
@@ -1435,4 +1428,4 @@ def remove(self, _, atoms=None, selection=None):
         """Remove selected atoms."""
         del [atoms[selection]]
 
-        self.structure, self.selection = atoms, list()
+        self.structure, self.selection = atoms, []