Skip to content

Commit

Permalink
fix/ignore some mypy errors, still some left
Browse files Browse the repository at this point in the history
  • Loading branch information
janosh committed Jan 9, 2023
1 parent e3e9cc3 commit 0a48ae2
Show file tree
Hide file tree
Showing 9 changed files with 39 additions and 31 deletions.
2 changes: 1 addition & 1 deletion ADMIN.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ following assumes you are using miniconda or Anaconda.
The general procedure to releasing pymatgen comprises the following
steps:

1. Wait for all unittests to pass on CircleCI.
1. Make sure all CI checks are green. We don't want to release known bugs.
2. Update and edit change log.
3. Release PyPI versions + doc.
4. Release conda versions.
Expand Down
17 changes: 11 additions & 6 deletions pymatgen/analysis/chemenv/connectivity/connected_components.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
Connected components.
"""

from __future__ import annotations

import itertools
import logging

Expand Down Expand Up @@ -222,7 +224,7 @@ def __init__(
Returns:
ConnectedComponent: Instance of this class
"""
self._periodicity_vectors = None
self._periodicity_vectors: list[list] | None = None
self._primitive_reduced_connected_subgraph = None
self._projected = False
if graph is None:
Expand Down Expand Up @@ -420,7 +422,7 @@ def compute_periodicity_all_simple_paths_algorithm(self):
paths = []
# TODO: its probably possible to do just a dfs or bfs traversal instead of taking all simple paths!
test_node_neighbors = my_simple_graph.neighbors(test_node)
breaknodeloop = False
break_node_loop = False
for test_node_neighbor in test_node_neighbors:
# Special case for two nodes
if len(self._connected_subgraph[test_node][test_node_neighbor]) > 1:
Expand All @@ -442,7 +444,7 @@ def compute_periodicity_all_simple_paths_algorithm(self):
test_node_neighbor,
cutoff=len(self._connected_subgraph),
):
path_indices = [nodepath.isite for nodepath in path]
path_indices = [node_path.isite for node_path in path]
if path_indices == [test_node.isite, test_node_neighbor.isite]:
continue
path_indices.append(test_node.isite)
Expand All @@ -465,9 +467,9 @@ def compute_periodicity_all_simple_paths_algorithm(self):
this_node_cell_img_vectors.extend(this_path_deltas)
this_node_cell_img_vectors = get_linearly_independent_vectors(this_node_cell_img_vectors)
if len(this_node_cell_img_vectors) == 3:
breaknodeloop = True
break_node_loop = True
break
if breaknodeloop:
if break_node_loop:
break
this_node_cell_img_vectors = get_linearly_independent_vectors(this_node_cell_img_vectors)
independent_cell_img_vectors = this_node_cell_img_vectors
Expand Down Expand Up @@ -504,7 +506,6 @@ def compute_periodicity_cycle_basis(self):
all_deltas.extend(this_cycle_deltas)
all_deltas = get_linearly_independent_vectors(all_deltas)
if len(all_deltas) == 3:
self._periodicity_vectors = all_deltas
return
# One has to consider pairs of nodes with parallel edges (these are not considered in the simple graph cycles)
edges = my_simple_graph.edges()
Expand Down Expand Up @@ -603,6 +604,7 @@ def is_0d(self) -> bool:
"""
if self._periodicity_vectors is None:
self.compute_periodicity()
assert self._periodicity_vectors is not None # fix mypy arg 1 to len has incompatible type Optional
return len(self._periodicity_vectors) == 0

@property
Expand All @@ -612,6 +614,7 @@ def is_1d(self) -> bool:
"""
if self._periodicity_vectors is None:
self.compute_periodicity()
assert self._periodicity_vectors is not None # fix mypy arg 1 to len has incompatible type Optional
return len(self._periodicity_vectors) == 1

@property
Expand All @@ -621,6 +624,7 @@ def is_2d(self) -> bool:
"""
if self._periodicity_vectors is None:
self.compute_periodicity()
assert self._periodicity_vectors is not None # fix mypy arg 1 to len has incompatible type Optional
return len(self._periodicity_vectors) == 2

@property
Expand All @@ -630,6 +634,7 @@ def is_3d(self) -> bool:
"""
if self._periodicity_vectors is None:
self.compute_periodicity()
assert self._periodicity_vectors is not None # fix mypy arg 1 to len has incompatible type Optional
return len(self._periodicity_vectors) == 3

@staticmethod
Expand Down
2 changes: 1 addition & 1 deletion pymatgen/analysis/magnetism/analyzer.py
Original file line number Diff line number Diff line change
Expand Up @@ -858,7 +858,7 @@ def _generate_transformations(self, structure: Structure) -> dict[str, MagOrderi
fm_structure = analyzer.get_ferromagnetic_structure()
# store magmom as spin property, to be consistent with output from
# other transformations
fm_structure.add_spin_by_site(fm_structure.site_properties["magmom"])
fm_structure.add_spin_by_site(fm_structure.site_properties["magmom"]) # type: ignore[arg-type]
fm_structure.remove_site_property("magmom")

# we now have our first magnetic ordering...
Expand Down
16 changes: 8 additions & 8 deletions pymatgen/core/operations.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ def operate(self, point: ArrayLike) -> np.ndarray:
Returns:
Coordinates of point after operation.
"""
affine_point = np.array([point[0], point[1], point[2], 1])
affine_point = np.array([*point, 1]) # type: ignore
return np.dot(self.affine_matrix, affine_point)[0:3]

def operate_multi(self, points: ArrayLike) -> np.ndarray:
Expand Down Expand Up @@ -577,7 +577,7 @@ class or as list or np array-like
return Magmom.from_global_moment_and_saxis(transformed_moment, magmom.saxis)

@classmethod
def from_symmop(cls, symmop, time_reversal) -> MagSymmOp:
def from_symmop(cls, symmop: SymmOp, time_reversal) -> MagSymmOp:
"""
Initialize a MagSymmOp from a SymmOp and time reversal operator.
Expand All @@ -588,8 +588,8 @@ def from_symmop(cls, symmop, time_reversal) -> MagSymmOp:
Returns:
MagSymmOp object
"""
magsymmop = cls(symmop.affine_matrix, time_reversal, symmop.tol)
return magsymmop
mag_symmop = cls(symmop.affine_matrix, time_reversal, symmop.tol)
return mag_symmop

@staticmethod
def from_rotation_and_translation_and_time_reversal(
Expand All @@ -611,10 +611,10 @@ def from_rotation_and_translation_and_time_reversal(
Returns:
MagSymmOp object
"""
symmop = SymmOp.from_rotation_and_translation(
symm_op = SymmOp.from_rotation_and_translation(
rotation_matrix=rotation_matrix, translation_vec=translation_vec, tol=tol
)
return MagSymmOp.from_symmop(symmop, time_reversal)
return MagSymmOp.from_symmop(symm_op, time_reversal)

@staticmethod
def from_xyzt_string(xyzt_string: str) -> MagSymmOp:
Expand All @@ -626,12 +626,12 @@ def from_xyzt_string(xyzt_string: str) -> MagSymmOp:
Returns:
MagSymmOp object
"""
symmop = SymmOp.from_xyz_string(xyzt_string.rsplit(",", 1)[0])
symm_op = SymmOp.from_xyz_string(xyzt_string.rsplit(",", 1)[0])
try:
time_reversal = int(xyzt_string.rsplit(",", 1)[1])
except Exception:
raise Exception("Time reversal operator could not be parsed.")
return MagSymmOp.from_symmop(symmop, time_reversal)
return MagSymmOp.from_symmop(symm_op, time_reversal)

def as_xyzt_string(self) -> str:
"""
Expand Down
8 changes: 4 additions & 4 deletions pymatgen/core/structure.py
Original file line number Diff line number Diff line change
Expand Up @@ -309,18 +309,18 @@ def atomic_numbers(self) -> tuple[int, ...]:
raise AttributeError("atomic_numbers available only for ordered Structures")

@property
def site_properties(self) -> dict[str, list]:
def site_properties(self) -> dict[str, Sequence]:
"""
Returns the site properties as a dict of sequences. E.g.,
{"magmom": (5,-5), "charge": (-4,4)}.
"""
props: dict[str, list] = {}
props: dict[str, Sequence] = {}
prop_keys: set[str] = set()
for site in self:
prop_keys.update(site.properties)

for k in prop_keys:
props[k] = [site.properties.get(k, None) for site in self]
for key in prop_keys:
props[key] = [site.properties.get(key) for site in self]
return props

def __contains__(self, site: object) -> bool:
Expand Down
12 changes: 7 additions & 5 deletions pymatgen/core/trajectory.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
import warnings
from fnmatch import fnmatch
from pathlib import Path
from typing import Any, Dict, List, Tuple, Union
from typing import Any, Dict, List, Sequence, Tuple, Union

import numpy as np
from monty.io import zopen
Expand All @@ -34,7 +34,7 @@

Vector3D = Tuple[float, float, float]
Matrix3D = Tuple[Vector3D, Vector3D, Vector3D]
SitePropsType = Union[List[Dict[Any, List[Any]]], Dict[Any, List[Any]]]
SitePropsType = Union[List[Dict[Any, Sequence[Any]]], Dict[Any, Sequence[Any]]]


class Trajectory(MSONable):
Expand Down Expand Up @@ -445,7 +445,7 @@ def from_structures(
lattice,
species, # type: ignore
frac_coords,
site_properties=site_properties,
site_properties=site_properties, # type: ignore
constant_lattice=constant_lattice,
**kwargs,
)
Expand Down Expand Up @@ -503,7 +503,9 @@ def _combine_lattice(lat1: np.ndarray, lat2: np.ndarray, len1: int, len2: int) -
return lat, constant_lat

@staticmethod
def _combine_site_props(prop1: SitePropsType | None, prop2: SitePropsType | None, len1: int, len2: int):
def _combine_site_props(
prop1: SitePropsType | None, prop2: SitePropsType | None, len1: int, len2: int
) -> SitePropsType | None:
"""
Combine site properties.
Expand All @@ -515,7 +517,7 @@ def _combine_site_props(prop1: SitePropsType | None, prop2: SitePropsType | None
if prop1 is None and prop2 is None:
return None

if isinstance(prop1, dict) and isinstance(prop2, dict) and prop1 == prop2:
if isinstance(prop1, dict) and prop1 == prop2:
return prop1

# general case
Expand Down
2 changes: 1 addition & 1 deletion pymatgen/io/lammps/data.py
Original file line number Diff line number Diff line change
Expand Up @@ -845,7 +845,7 @@ def from_structure(cls, structure: Structure, ff_elements=None, atom_style="char
velos = np.array(s.site_properties["velocities"])
rot = SymmOp.from_rotation_and_translation(symm_op.rotation_matrix)
rot_velos = rot.operate_multi(velos)
site_properties.update({"velocities": rot_velos})
site_properties.update({"velocities": rot_velos}) # type: ignore
boxed_s = Structure(
box.to_lattice(),
s.species,
Expand Down
6 changes: 3 additions & 3 deletions pymatgen/symmetry/analyzer.py
Original file line number Diff line number Diff line change
Expand Up @@ -757,9 +757,9 @@ def get_conventional_standard_structure(self, international_monoclinic=True, kee
],
]

def is_all_acute_or_obtuse(m) -> bool:
recp_angles = np.array(Lattice(m).reciprocal_lattice.angles)
return np.all(recp_angles <= 90) or np.all(recp_angles > 90)
def is_all_acute_or_obtuse(matrix) -> bool:
recp_angles = np.array(Lattice(matrix).reciprocal_lattice.angles)
return all(recp_angles <= 90) or all(recp_angles > 90)

if is_all_acute_or_obtuse(test_matrix):
transf = np.eye(3)
Expand Down
5 changes: 3 additions & 2 deletions pymatgen/util/coord.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,8 @@ def is_coord_subset(subset, superset, atol=1e-8) -> bool:
Doesn't use periodic boundary conditions
Args:
subset, superset: List of coords
subset: List of coords
superset: List of coords
Returns:
True if all of subset is in superset.
Expand All @@ -76,7 +77,7 @@ def is_coord_subset(subset, superset, atol=1e-8) -> bool:
c2 = np.array(superset)
is_close = np.all(np.abs(c1[:, None, :] - c2[None, :, :]) < atol, axis=-1)
any_close = np.any(is_close, axis=-1)
return np.all(any_close)
return all(any_close)


def coord_list_mapping(subset, superset, atol=1e-8):
Expand Down

0 comments on commit 0a48ae2

Please sign in to comment.