Skip to content

Commit

Permalink
ruff fix FURB142: set.add() in a for loop and a few ruff DOC502
Browse files Browse the repository at this point in the history
  • Loading branch information
janosh committed Aug 16, 2024
1 parent d9f5705 commit 6427b55
Show file tree
Hide file tree
Showing 11 changed files with 67 additions and 65 deletions.
1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ repair-wheel-command = "delocate-wheel --require-archs {delocate_archs} -w {dest
[tool.ruff]
target-version = "py39"
line-length = 120
output-format = "concise"

[tool.ruff.lint]
select = ["ALL"]
Expand Down
12 changes: 6 additions & 6 deletions src/pymatgen/core/lattice.py
Original file line number Diff line number Diff line change
Expand Up @@ -1803,17 +1803,17 @@ def get_points_in_spheres(

# Temporarily hold the fractional coordinates
image_offsets = lattice.get_fractional_coords(all_coords)
all_fcoords = []
all_frac_coords = []

# Only wrap periodic boundary
for kk in range(3):
if _pbc[kk]:
all_fcoords.append(np.mod(image_offsets[:, kk : kk + 1], 1))
all_frac_coords.append(np.mod(image_offsets[:, kk : kk + 1], 1))
else:
all_fcoords.append(image_offsets[:, kk : kk + 1])
all_fcoords = np.concatenate(all_fcoords, axis=1)
image_offsets = image_offsets - all_fcoords
coords_in_cell = np.dot(all_fcoords, matrix)
all_frac_coords.append(image_offsets[:, kk : kk + 1])
all_frac_coords = np.concatenate(all_frac_coords, axis=1)
image_offsets = image_offsets - all_frac_coords
coords_in_cell = np.dot(all_frac_coords, matrix)

# Filter out those beyond max range
valid_coords = []
Expand Down
16 changes: 8 additions & 8 deletions src/pymatgen/core/structure.py
Original file line number Diff line number Diff line change
Expand Up @@ -2159,8 +2159,8 @@ def get_all_neighbors_old(
lattice = self._lattice
matrix = lattice.matrix
neighbors: list[list] = [[] for _ in range(len(self))]
all_fcoords = np.mod(self.frac_coords, 1)
coords_in_cell = np.dot(all_fcoords, matrix)
all_frac_coords = np.mod(self.frac_coords, 1)
coords_in_cell = np.dot(all_frac_coords, matrix)
site_coords = self.cart_coords

indices = np.arange(len(self))
Expand Down Expand Up @@ -2519,14 +2519,14 @@ def site_label(site):
# Group sites by species string
sites = sorted(self._sites, key=site_label)

grouped_sites = [list(a[1]) for a in itertools.groupby(sites, key=site_label)]
grouped_sites = [list(grp) for _, grp in itertools.groupby(sites, key=site_label)]
grouped_frac_coords = [np.array([s.frac_coords for s in g]) for g in grouped_sites]

# min_vecs are approximate periodicities of the cell. The exact
# periodicities from the supercell matrices are checked against these
# first
min_fcoords = min(grouped_frac_coords, key=len)
min_vecs = min_fcoords - min_fcoords[0]
min_frac_coords = min(grouped_frac_coords, key=len)
min_vecs = min_frac_coords - min_frac_coords[0]

# Fractional tolerance in the supercell
super_ftol = np.divide(tolerance, self.lattice.abc)
Expand Down Expand Up @@ -2636,14 +2636,14 @@ def factors(n: int):

# Add the new sites, averaging positions
added = np.zeros(len(gsites))
new_fcoords = all_frac % 1
new_frac_coords = all_frac % 1
for grp_idx, group in enumerate(groups):
if not added[grp_idx]:
added[group] = True
inds = np.where(group)[0]
coords = new_fcoords[inds[0]]
coords = new_frac_coords[inds[0]]
for inner_idx, ind in enumerate(inds[1:]):
offset = new_fcoords[ind] - coords
offset = new_frac_coords[ind] - coords
coords += (offset - np.round(offset)) / (inner_idx + 2)
new_sp.append(gsites[inds[0]].species)
for k in gsites[inds[0]].properties:
Expand Down
6 changes: 3 additions & 3 deletions src/pymatgen/core/surface.py
Original file line number Diff line number Diff line change
Expand Up @@ -828,10 +828,10 @@ def get_slab_regions(
# If slab is noncontiguous
if frac_coords:
# Locate the lowest site within the upper Slab
last_fcoords = []
last_frac_coords = []
last_indices = []
while frac_coords:
last_fcoords = copy.copy(frac_coords)
last_frac_coords = copy.copy(frac_coords)
last_indices = copy.copy(indices)

site = slab[indices[frac_coords.index(min(frac_coords))]]
Expand All @@ -850,7 +850,7 @@ def get_slab_regions(
for site in slab:
if all(nn.index not in all_indices for nn in slab.get_neighbors(site, blength)):
upper_fcoords.append(site.frac_coords[2])
coords: list = copy.copy(frac_coords) if frac_coords else copy.copy(last_fcoords)
coords: list = copy.copy(frac_coords) if frac_coords else copy.copy(last_frac_coords)
min_top = slab[last_indices[coords.index(min(coords))]].frac_coords[2]
return [(0, max(upper_fcoords)), (min_top, 1)]

Expand Down
9 changes: 5 additions & 4 deletions src/pymatgen/core/trajectory.py
Original file line number Diff line number Diff line change
Expand Up @@ -699,10 +699,11 @@ def _check_site_props(self, site_props: SitePropsType | None) -> None:
n_sites = len(self.coords[0])
for dct in site_props:
for key, val in dct.items():
assert len(val) == n_sites, (
f"Size of site property {key} {len(val)}) does not equal the "
f"number of sites in the structure {n_sites}."
)
if len(val) != n_sites:
raise ValueError(
f"Size of site property {key} {len(val)}) does not equal the "
f"number of sites in the structure {n_sites}."
)

def _check_frame_props(self, frame_props: list[dict] | None) -> None:
"""Check data shape of site properties."""
Expand Down
9 changes: 3 additions & 6 deletions src/pymatgen/io/multiwfn.py
Original file line number Diff line number Diff line change
Expand Up @@ -448,8 +448,7 @@ def sort_cps_by_distance(
# Add all unique atoms involved in this ring
atom_inds = set()
for bond_name in bond_names:
for atom_ind in bond_cps[bond_name]["atom_inds"]:
atom_inds.add(atom_ind)
atom_inds.update(bond_cps[bond_name]["atom_inds"])

modified_organized_cps["ring"][cp_name]["bond_names"] = bond_names
modified_organized_cps["ring"][cp_name]["atom_inds"] = list(atom_inds)
Expand Down Expand Up @@ -477,10 +476,8 @@ def sort_cps_by_distance(
bond_names_cage = set()
atom_inds = set()
for ring_name in ring_names:
for bond_name in ring_cps[ring_name]["bond_names"]:
bond_names_cage.add(bond_name) # type: ignore[attr-defined]
for atom_ind in ring_cps[ring_name]["atom_inds"]:
atom_inds.add(atom_ind) # type: ignore[attr-defined]
bond_names_cage.update(ring_cps[ring_name]["bond_names"]) # type: ignore[attr-defined]
atom_inds.update(ring_cps[ring_name]["atom_inds"]) # type: ignore[attr-defined]

modified_organized_cps["cage"][cp_name]["ring_names"] = ring_names
modified_organized_cps["cage"][cp_name]["bond_names"] = list(bond_names_cage)
Expand Down
14 changes: 7 additions & 7 deletions src/pymatgen/optimization/neighbors.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ def find_points_in_spheres(
double [:, ::1] frac_coords = <double[:n_center, :3]> safe_malloc(
n_center * 3 * sizeof(double)
)
double[:, ::1] all_fcoords = <double[:n_total, :3]> safe_malloc(
double[:, ::1] all_frac_coords = <double[:n_total, :3]> safe_malloc(
n_total * 3 * sizeof(double)
)
double[:, ::1] coords_in_cell = <double[:n_total, :3]> safe_malloc(
Expand Down Expand Up @@ -149,10 +149,10 @@ def find_points_in_spheres(
for j in range(3):
if pbc[j]:
# only wrap atoms when this dimension is PBC
all_fcoords[i, j] = offset_correction[i, j] % 1
offset_correction[i, j] = offset_correction[i, j] - all_fcoords[i, j]
all_frac_coords[i, j] = offset_correction[i, j] % 1
offset_correction[i, j] = offset_correction[i, j] - all_frac_coords[i, j]
else:
all_fcoords[i, j] = offset_correction[i, j]
all_frac_coords[i, j] = offset_correction[i, j]
offset_correction[i, j] = 0

# compute the reciprocal lattice in place
Expand All @@ -165,7 +165,7 @@ def find_points_in_spheres(

for i in range(3):
nlattice *= (max_bounds[i] - min_bounds[i])
matmul(all_fcoords, lattice, coords_in_cell)
matmul(all_frac_coords, lattice, coords_in_cell)

# Get translated images, coordinates and indices
for i in range(min_bounds[0], max_bounds[0]):
Expand Down Expand Up @@ -229,7 +229,7 @@ def find_points_in_spheres(
# if no valid neighbors were found return empty
if count == 0:
free(&frac_coords[0, 0])
free(&all_fcoords[0, 0])
free(&all_frac_coords[0, 0])
free(&coords_in_cell[0, 0])
free(&offset_correction[0, 0])
free(&center_indices1[0])
Expand Down Expand Up @@ -379,7 +379,7 @@ def find_points_in_spheres(

free(&offset_correction[0, 0])
free(&frac_coords[0, 0])
free(&all_fcoords[0, 0])
free(&all_frac_coords[0, 0])
free(&coords_in_cell[0, 0])
free(&center_indices1[0])
free(&center_indices3[0, 0])
Expand Down
12 changes: 7 additions & 5 deletions src/pymatgen/symmetry/groups.py
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@ def from_space_group(cls, sg_symbol: str) -> PointGroup:
Raises:
AssertionError if a valid crystal class cannot be created
Returns:
crystal class in Hermann-Mauguin notation.
"""
Expand All @@ -202,18 +203,19 @@ def from_space_group(cls, sg_symbol: str) -> PointGroup:
if symbol in [spg["hermann_mauguin"], spg["universal_h_m"], spg["hermann_mauguin_u"]]:
symbol = spg["short_h_m"]

assert symbol[0].isupper(), f"Invalid sg_symbol {sg_symbol}"
assert not symbol[1:].isupper(), f"Invalid sg_symbol {sg_symbol}"
if not symbol[0].isupper():
raise ValueError(f"Invalid {sg_symbol=}")
if symbol[1:].isupper():
raise ValueError(f"Invalid {sg_symbol=}")

symbol = symbol[1:] # Remove centering
symbol = symbol.translate(str.maketrans("abcden", "mmmmmm")) # Remove translation from glide planes
symbol = re.sub(r"_.", "", symbol) # Remove translation from screw axes
symbol = abbrev_map.get(symbol, symbol)
symbol = non_standard_map.get(symbol, symbol)

assert (
symbol in SYMM_DATA["point_group_encoding"]
), f"Could not create a valid crystal class ({symbol}) from sg_symbol {sg_symbol}"
if symbol not in SYMM_DATA["point_group_encoding"]:
raise ValueError(f"Could not create a valid crystal class ({symbol}) from {sg_symbol=}")
return cls(symbol)


Expand Down
7 changes: 4 additions & 3 deletions src/pymatgen/util/testing/aims.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ def comp_system(
user_kpt_settings (dict[str, Any] | None): settings for k-point density in FHI-aims
Raises:
AssertionError: If the input files are not the same
ValueError: If the input files are not the same
"""
if user_kpt_settings is None:
user_kpt_settings = {}
Expand Down Expand Up @@ -130,7 +130,7 @@ def compare_single_files(ref_file: str | Path, test_file: str | Path) -> None:
test_file (str | Path): The file to compare against the reference
Raises:
AssertionError: If the files are not the same
ValueError: If the files are not the same
"""
with open(test_file) as tf:
test_lines = tf.readlines()[5:]
Expand All @@ -141,7 +141,8 @@ def compare_single_files(ref_file: str | Path, test_file: str | Path) -> None:
for test_line, ref_line in zip(test_lines, ref_lines):
if "species_dir" in ref_line:
continue
assert test_line.strip() == ref_line.strip()
if test_line.strip() != ref_line.strip():
raise ValueError(f"{test_line=} != {ref_line=}")


Si = Structure(
Expand Down
24 changes: 12 additions & 12 deletions src/pymatgen/vis/structure_vtk.py
Original file line number Diff line number Diff line change
Expand Up @@ -1033,10 +1033,10 @@ def apply_tags(self):
if cell_index == (0, 0, 0):
coords = site.coords
else:
fcoords = site.frac_coords + np.array(cell_index)
frac_coords = site.frac_coords + np.array(cell_index)
site_image = PeriodicSite(
site.species,
fcoords,
frac_coords,
self.current_structure.lattice,
to_unit_cell=False,
coords_are_cartesian=False,
Expand Down Expand Up @@ -1097,20 +1097,20 @@ def display_warning(self, warning):
warning (str): Warning.
"""
self.warning_txt_mapper = vtk.vtkTextMapper()
tprops = self.warning_txt_mapper.GetTextProperty()
tprops.SetFontSize(14)
tprops.SetFontFamilyToTimes()
tprops.SetColor(1, 0, 0)
tprops.BoldOn()
tprops.SetJustificationToRight()
text_props = self.warning_txt_mapper.GetTextProperty()
text_props.SetFontSize(14)
text_props.SetFontFamilyToTimes()
text_props.SetColor(1, 0, 0)
text_props.BoldOn()
text_props.SetJustificationToRight()
self.warning_txt = f"WARNING : {warning}"
self.warning_txt_actor = vtk.vtkActor2D()
self.warning_txt_actor.VisibilityOn()
self.warning_txt_actor.SetMapper(self.warning_txt_mapper)
self.ren.AddActor(self.warning_txt_actor)
self.warning_txt_mapper.SetInput(self.warning_txt)
winsize = self.ren_win.GetSize()
self.warning_txt_actor.SetPosition(winsize[0] - 10, 10)
win_size = self.ren_win.GetSize()
self.warning_txt_actor.SetPosition(win_size[0] - 10, 10)
self.warning_txt_actor.VisibilityOn()

def erase_warning(self):
Expand All @@ -1135,8 +1135,8 @@ def display_info(self, info):
self.info_txt_actor.SetMapper(self.info_txt_mapper)
self.ren.AddActor(self.info_txt_actor)
self.info_txt_mapper.SetInput(self.info_txt)
winsize = self.ren_win.GetSize()
self.info_txt_actor.SetPosition(10, winsize[1] - 10)
win_size = self.ren_win.GetSize()
self.info_txt_actor.SetPosition(10, win_size[1] - 10)
self.info_txt_actor.VisibilityOn()

def erase_info(self):
Expand Down
22 changes: 11 additions & 11 deletions tests/core/test_lattice.py
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,7 @@ def test_get_points_in_sphere(self):
assert types == {np.ndarray}, f"Expected only np.ndarray, got {types}"

def test_get_all_distances(self):
fcoords = np.array(
frac_coords = np.array(
[
[0.3, 0.3, 0.5],
[0.1, 0.1, 0.3],
Expand All @@ -409,10 +409,10 @@ def test_get_all_distances(self):
[3.245, 4.453, 1.788, 3.852, 0.000],
]
)
output = lattice.get_all_distances(fcoords, fcoords)
output = lattice.get_all_distances(frac_coords, frac_coords)
assert_allclose(output, expected, 3)
# test just one input point
output2 = lattice.get_all_distances(fcoords[0], fcoords)
output2 = lattice.get_all_distances(frac_coords[0], frac_coords)
assert_allclose(output2, [expected[0]], 2)
# test distance when initial points are not in unit cell
f1 = [0, 0, 17]
Expand All @@ -429,7 +429,7 @@ def test_get_all_distances(self):
[3.519, 1.131, 2.251, 0.000, 4.235],
]
)
output3 = lattice_pbc.get_all_distances(fcoords[:-1], fcoords)
output3 = lattice_pbc.get_all_distances(frac_coords[:-1], frac_coords)
assert_allclose(output3, expected_pbc, 3)

def test_monoclinic(self):
Expand Down Expand Up @@ -481,20 +481,20 @@ def test_lll_basis(self):
l2 = Lattice([a + b, b + c, c])

cart_coords = np.array([[1, 1, 2], [2, 2, 1.5]])
l1_fcoords = l1.get_fractional_coords(cart_coords)
l2_fcoords = l2.get_fractional_coords(cart_coords)
l1_frac_coords = l1.get_fractional_coords(cart_coords)
l2_frac_coords = l2.get_fractional_coords(cart_coords)

assert_allclose(l1.matrix, l2.lll_matrix)
assert_allclose(np.dot(l2.lll_mapping, l2.matrix), l1.matrix)

assert_allclose(np.dot(l2_fcoords, l2.matrix), np.dot(l1_fcoords, l1.matrix))
assert_allclose(np.dot(l2_frac_coords, l2.matrix), np.dot(l1_frac_coords, l1.matrix))

lll_fcoords = l2.get_lll_frac_coords(l2_fcoords)
lll_frac_coords = l2.get_lll_frac_coords(l2_frac_coords)

assert_allclose(lll_fcoords, l1_fcoords)
assert_allclose(l1.get_cartesian_coords(lll_fcoords), np.dot(lll_fcoords, l2.lll_matrix))
assert_allclose(lll_frac_coords, l1_frac_coords)
assert_allclose(l1.get_cartesian_coords(lll_frac_coords), np.dot(lll_frac_coords, l2.lll_matrix))

assert_allclose(l2.get_frac_coords_from_lll(lll_fcoords), l2_fcoords)
assert_allclose(l2.get_frac_coords_from_lll(lll_frac_coords), l2_frac_coords)

def test_get_miller_index_from_sites(self):
# test on a cubic system
Expand Down

0 comments on commit 6427b55

Please sign in to comment.