Merge pull request #2551 from janosh/flake8-bugbear

Add `flake8` plugin `bugbear` as pre-commit hook

.pre-commit-config.yaml

@@ -38,7 +38,7 @@ repos:
     rev: 5.10.1
     hooks:
       - id: isort
-        args: ["--profile", "black"]
+        args: [--profile, black]
 
   - repo: https://github.com/psf/black
     rev: 22.6.0
@@ -49,6 +49,7 @@ repos:
     rev: 4.0.1
     hooks:
       - id: flake8
+        additional_dependencies: [flake8-bugbear]
 
   - repo: https://github.com/pre-commit/mirrors-mypy
     rev: v0.961

dev_scripts/update_pt_data.py

@@ -271,7 +271,7 @@ def add_electron_affinities():
         data.append(row)
     data.pop(0)
     ea = {int(r[0]): float(re.sub(r"[\s\(\)]", "", r[3].strip("()[]"))) for r in data}
-    assert set(ea.keys()).issuperset(range(1, 93))  # Ensure that we have data for up to U.
+    assert set(ea).issuperset(range(1, 93))  # Ensure that we have data for up to U.
     print(ea)
     pt = loadfn("../pymatgen/core/periodic_table.json")
     for k, v in pt.items():
@@ -305,7 +305,7 @@ def add_ionization_energies():
             data[Z].append(val)
     print(data)
     print(data[51])
-    assert set(data.keys()).issuperset(range(1, 93))  # Ensure that we have data for up to U.
+    assert set(data).issuperset(range(1, 93))  # Ensure that we have data for up to U.
     pt = loadfn("../pymatgen/core/periodic_table.json")
     for k, v in pt.items():
         del v["Ionization energy"]

pymatgen/alchemy/filters.py

@@ -149,7 +149,7 @@ def test(self, structure):
         Returns: True if structure does not contain species within specified
             distances.
         """
-        all_species = set(self.specie_and_min_dist.keys())
+        all_species = set(self.specie_and_min_dist)
         for site in structure:
             species = site.species.keys()
             sp_to_test = set(species).intersection(all_species)
@@ -190,11 +190,7 @@ class RemoveDuplicatesFilter(AbstractStructureFilter):
     This filter removes exact duplicate structures from the transmuter.
     """
 
-    def __init__(
-        self,
-        structure_matcher=StructureMatcher(comparator=ElementComparator()),
-        symprec=None,
-    ):
+    def __init__(self, structure_matcher=None, symprec=None):
         """
         Remove duplicate structures based on the structure matcher
         and symmetry (if symprec is given).
@@ -211,7 +207,7 @@ def __init__(
         if isinstance(structure_matcher, dict):
             self.structure_matcher = StructureMatcher.from_dict(structure_matcher)
         else:
-            self.structure_matcher = structure_matcher
+            self.structure_matcher = structure_matcher or StructureMatcher(comparator=ElementComparator())
 
     def test(self, structure):
         """
@@ -243,12 +239,7 @@ class RemoveExistingFilter(AbstractStructureFilter):
     This filter removes structures existing in a given list from the transmuter.
     """
 
-    def __init__(
-        self,
-        existing_structures,
-        structure_matcher=StructureMatcher(comparator=ElementComparator()),
-        symprec=None,
-    ):
+    def __init__(self, existing_structures, structure_matcher=None, symprec=None):
         """
         Remove existing structures based on the structure matcher
         and symmetry (if symprec is given).
@@ -267,7 +258,7 @@ def __init__(
         if isinstance(structure_matcher, dict):
             self.structure_matcher = StructureMatcher.from_dict(structure_matcher)
         else:
-            self.structure_matcher = structure_matcher
+            self.structure_matcher = structure_matcher or StructureMatcher(comparator=ElementComparator())
 
     def test(self, structure):
         """

pymatgen/alchemy/materials.py

@@ -294,7 +294,7 @@ def from_cif_string(
         parser = CifParser.from_string(cif_string, occupancy_tolerance)
         raw_string = re.sub(r"'", '"', cif_string)
         cif_dict = parser.as_dict()
-        cif_keys = list(cif_dict.keys())
+        cif_keys = list(cif_dict)
         s = parser.get_structures(primitive)[0]
         partial_cif = cif_dict[cif_keys[0]]
         if "_database_code_ICSD" in partial_cif:

pymatgen/analysis/bond_dissociation.py

@@ -5,6 +5,8 @@
 Module for BondDissociationEnergies.
 """
 
+from __future__ import annotations
+
 import logging
 
 import networkx as nx

pymatgen/analysis/bond_valence.py

@@ -208,7 +208,7 @@ def _calc_site_probabilities(self, site, nn):
     def _calc_site_probabilities_unordered(self, site, nn):
         bv_sum = calculate_bv_sum_unordered(site, nn, scale_factor=self.dist_scale_factor)
         prob = {}
-        for specie, occu in site.species.items():
+        for specie in site.species:
             el = specie.symbol
 
             prob[el] = {}
@@ -246,7 +246,7 @@ def get_valences(self, structure):
         """
         els = [Element(el.symbol) for el in structure.composition.elements]
 
-        if not set(els).issubset(set(BV_PARAMS.keys())):
+        if not set(els).issubset(set(BV_PARAMS)):
             raise ValueError("Structure contains elements not in set of BV parameters!")
 
         # Perform symmetry determination and get sites grouped by symmetry.
@@ -270,7 +270,7 @@ def get_valences(self, structure):
                 nn = structure.get_neighbors(test_site, self.max_radius)
                 prob = self._calc_site_probabilities(test_site, nn)
                 all_prob.append(prob)
-                val = list(prob.keys())
+                val = list(prob)
                 # Sort valences in order of decreasing probability.
                 val = sorted(val, key=lambda v: -prob[v])
                 # Retain probabilities that are at least 1/100 of highest prob.
@@ -284,8 +284,8 @@ def get_valences(self, structure):
                 all_prob.append(prob)
                 full_all_prob.extend(prob.values())
                 vals = []
-                for (elsp, occ) in get_z_ordered_elmap(test_site.species):
-                    val = list(prob[elsp.symbol].keys())
+                for (elsp, _) in get_z_ordered_elmap(test_site.species):
+                    val = list(prob[elsp.symbol])
                     # Sort valences in order of decreasing probability.
                     val = sorted(val, key=lambda v: -prob[elsp.symbol][v])
                     # Retain probabilities that are at least 1/100 of highest
@@ -322,11 +322,13 @@ def evaluate_assignment(v_set):
                     self._best_vset = v_set
                     self._best_score = score
 
-            def _recurse(assigned=[]):
+            def _recurse(assigned=None):
                 # recurses to find permutations of valences based on whether a
                 # charge balanced assignment can still be found
                 if self._n > self.max_permutations:
                     return None
+                if assigned is None:
+                    assigned = []
 
                 i = len(assigned)
                 highest = vmax.copy()
@@ -357,7 +359,7 @@ def _recurse(assigned=[]):
             tmp = []
             attrib = []
             for insite, nsite in enumerate(nsites):
-                for val in valences[insite]:
+                for _ in valences[insite]:
                     tmp.append(nsite)
                     attrib.append(insite)
             new_nsites = np.array(tmp)
@@ -382,8 +384,8 @@ def _recurse(assigned=[]):
             def evaluate_assignment(v_set):
                 el_oxi = collections.defaultdict(list)
                 jj = 0
-                for i, sites in enumerate(equi_sites):
-                    for specie, occu in get_z_ordered_elmap(sites[0].species):
+                for sites in equi_sites:
+                    for specie, _ in get_z_ordered_elmap(sites[0].species):
                         el_oxi[specie.symbol].append(v_set[jj])
                         jj += 1
                 max_diff = max(max(v) - min(v) for v in el_oxi.values())
@@ -398,11 +400,13 @@ def evaluate_assignment(v_set):
                     self._best_vset = v_set
                     self._best_score = score
 
-            def _recurse(assigned=[]):
+            def _recurse(assigned=None):
                 # recurses to find permutations of valences based on whether a
                 # charge balanced assignment can still be found
                 if self._n > self.max_permutations:
                     return None
+                if assigned is None:
+                    assigned = []
 
                 i = len(assigned)
                 highest = vmax.copy()
@@ -444,7 +448,7 @@ def _recurse(assigned=[]):
                 return [int(assigned[site]) for site in structure]
             assigned = {}
             new_best_vset = []
-            for ii in range(len(equi_sites)):
+            for _ in equi_sites:
                 new_best_vset.append([])
             for ival, val in enumerate(self._best_vset):
                 new_best_vset[attrib[ival]].append(val)
@@ -492,7 +496,7 @@ def get_z_ordered_elmap(comp):
     Cr4+, Cr3+, Ni3+, Ni4+, Zn2+ ... or
     Cr4+, Cr3+, Ni4+, Ni3+, Zn2+
     """
-    return sorted((elsp, comp[elsp]) for elsp in comp.keys())
+    return sorted((elsp, comp[elsp]) for elsp in comp)
 
 
 def add_oxidation_state_by_site_fraction(structure, oxidation_states):

pymatgen/analysis/chemenv/connectivity/connectivity_finder.py

@@ -2,6 +2,8 @@
 Module implementing connectivity finding.
 """
 
+from __future__ import annotations
+
 import logging
 
 import numpy as np
@@ -74,7 +76,6 @@ def setup_parameters(self, multiple_environments_choice):
         if multiple_environments_choice is not None:
             if multiple_environments_choice not in ["TAKE_HIGHEST_FRACTION"]:
                 raise ValueError(
-                    'Option "{}" for multiple_environments_choice is '
-                    "not allowed".format(multiple_environments_choice)
+                    f'Option "{multiple_environments_choice}" for multiple_environments_choice is not allowed'
                 )
         self.multiple_environments_choice = multiple_environments_choice

pymatgen/analysis/chemenv/coordination_environments/coordination_geometry_finder.py

@@ -160,26 +160,25 @@ def __str__(self):
                 outs.append(
                     "Points are referenced to the central site for coordination numbers < 5"
                     " and to the centroid (calculated with the central site) for coordination"
-                    " numbers >= 5 : {c}\n".format(c=self.centre)
+                    f" numbers >= 5 : {self.centre}\n"
                 )
             else:
                 outs.append(
                     "Points are referenced to the central site for coordination numbers < 5"
                     " and to the centroid (calculated without the central site) for coordination"
-                    " numbers >= 5 : {c}\n".format(c=self.centre)
+                    f" numbers >= 5 : {self.centre}\n"
                 )
         elif self.centering_type == "central_site":
             outs.append(f"Points are referenced to the central site : {self.centre}\n")
         elif self.centering_type == "centroid":
             if self.include_central_site_in_centroid:
                 outs.append(
-                    "Points are referenced to the centroid "
-                    "(calculated with the central site) :\n  {c}\n".format(c=self.centre)
+                    f"Points are referenced to the centroid (calculated with the central site) :\n  {self.centre}\n"
                 )
             else:
                 outs.append(
-                    "Points are referenced to the centroid"
-                    " (calculated without the central site) :\n  {c}\n".format(c=self.centre)
+                    "Points are referenced to the centroid (calculated without the central site)"
+                    f" :\n  {self.centre}\n"
                 )
         return "\n".join(outs)
 
@@ -491,7 +490,7 @@ def get_structure(self):
         """
         return self.structure
 
-    def set_structure(self, lattice, species, coords, coords_are_cartesian):
+    def set_structure(self, lattice: Lattice, species, coords, coords_are_cartesian):
         """
         Sets up the pymatgen structure for which the coordination geometries have to be identified starting from the
         lattice, the species and the coordinates
@@ -742,8 +741,7 @@ def compute_structure_environments(
                 continue
             if breakit:
                 logging.debug(
-                    " ... in site #{:d}/{:d} ({}) : "
-                    "skipped (timelimit)".format(isite, len(self.structure), site.species_string)
+                    f" ... in site #{isite}/{len(self.structure)} ({site.species_string}) : skipped (timelimit)"
                 )
                 continue
             logging.debug(f" ... in site #{isite:d}/{len(self.structure):d} ({site.species_string})")
@@ -838,10 +836,7 @@ def compute_structure_environments(
                 cn_new_nb_set = missing_nb_set_to_add["cn_new_nb_set"]
                 new_nb_set = missing_nb_set_to_add["new_nb_set"]
                 inew_nb_set = se.neighbors_sets[isite_new_nb_set][cn_new_nb_set].index(new_nb_set)
-                logging.debug(
-                    "    ... getting environments for nb_set ({:d}, {:d}) - "
-                    "from hints".format(cn_new_nb_set, inew_nb_set)
-                )
+                logging.debug(f"    ... getting environments for nb_set ({cn_new_nb_set}, {inew_nb_set}) - from hints")
                 tnbset1 = time.process_time()
                 self.update_nb_set_environments(
                     se=se,
@@ -1107,14 +1102,8 @@ def setup_random_structure(self, coordination):
         aa = 0.4
         bb = -0.2
         coords = []
-        for ii in range(coordination + 1):
-            coords.append(
-                aa
-                * np.random.random_sample(
-                    3,
-                )
-                + bb
-            )
+        for _ in range(coordination + 1):
+            coords.append(aa * np.random.random_sample(3) + bb)
         self.set_structure(
             lattice=np.array([[10, 0, 0], [0, 10, 0], [0, 0, 10]], np.float_),
             species=["Si"] * (coordination + 1),
@@ -1299,7 +1288,7 @@ def get_coordination_symmetry_measures_optim(
             logging.log(
                 level=5,
                 msg="Getting Continuous Symmetry Measure with Separation Plane "
-                'algorithm for geometry "{}"'.format(geometry.ce_symbol),
+                f'algorithm for geometry "{geometry.ce_symbol}"',
             )
             self.perfect_geometry = AbstractGeometry.from_cg(
                 cg=geometry,
@@ -1632,7 +1621,7 @@ def coordination_geometry_symmetry_measures_separation_plane_optim(
         local2perfect_maps = []
 
         if separation_plane_algo.separation in nb_set.separations:
-            for sep_indices, (local_plane, npsep) in nb_set.separations[separation_plane_algo.separation].items():
+            for local_plane, npsep in nb_set.separations[separation_plane_algo.separation].values():
                 cgsm = cgcsmoptim(
                     coordination_geometry=coordination_geometry,
                     sepplane=separation_plane_algo,
@@ -1831,7 +1820,7 @@ def _cg_csm_separation_plane(
 
             # plane_found = True
 
-            for i_sep_perm, sep_perm in enumerate(sep_perms):
+            for sep_perm in sep_perms:
                 perm1 = [separation_perm[ii] for ii in sep_perm]
                 pp = [perm1[ii] for ii in argref_separation]
                 # Skip permutations that have already been performed
@@ -1918,7 +1907,7 @@ def _cg_csm_separation_plane_optim1(
         else:
             sep_perms = sepplane.permutations
 
-        for i_sep_perm, sep_perm in enumerate(sep_perms):
+        for sep_perm in sep_perms:
             perm1 = [separation_perm[ii] for ii in sep_perm]
             pp = [perm1[ii] for ii in argref_separation]
 
@@ -1993,7 +1982,7 @@ def _cg_csm_separation_plane_optim2(
         else:
             sep_perms = sepplane.permutations
 
-        for i_sep_perm, sep_perm in enumerate(sep_perms):
+        for sep_perm in sep_perms:
             perm1 = separation_perm.take(sep_perm)
             pp = perm1.take(argref_separation)