Skip to content

Commit

Permalink
Add CI run without 'optional' deps installed (#3857)
Browse files Browse the repository at this point in the history
* add CI run on macos-latest, python=3.12 with resolution: lowest and extras: '' to test pymatgen without any optional deps installed

* restore quotes around py version

* fix error: Failed to parse: `.[ci,]`
  Caused by: Couldn't parse requirement at position 5
  Caused by: Expected an alphanumeric character starting the extra name, found ']'
.[ci,]

* change uv resolution to lowest-direct on macos

* skip pwmat input tests that require seekpath if optional seekpath is not installed

* Trajectory class: raise ImportError with ASE installation instructions when reading .traj files

* fix TestTrajectory.test_from_file() when ase not installed, test expected error message

* Fix outdated NO_ASE_ERR attribute access in test_ase.py

* simplify ComputedEntry.__eq__

f-strings, better var names, fix ruff RUF002
  • Loading branch information
janosh authored Jun 3, 2024
1 parent 1a5af18 commit d1fac20
Show file tree
Hide file tree
Showing 20 changed files with 65 additions and 35 deletions.
18 changes: 15 additions & 3 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,20 @@ jobs:
# newest version (currently 3.12) on ubuntu (to get complete coverage on unix). We
# ignore mac-os, which is assumed to be similar to ubuntu.
config:
- { os: windows-latest, python: "3.9", resolution: highest }
- { os: ubuntu-latest, python: "3.12", resolution: lowest-direct }
- os: windows-latest
python: "3.9"
resolution: highest
extras: ci,optional
- os: ubuntu-latest
python: "3.12"
resolution: lowest-direct
extras: ci,optional
# test with lowest resolution and only required dependencies installed
- os: macos-latest
python: "3.10"
resolution: lowest-direct
extras: ci

# pytest-split automatically distributes work load so parallel jobs finish in similar time
split: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10]

Expand Down Expand Up @@ -69,7 +81,7 @@ jobs:
pip install torch
uv pip install numpy cython
uv pip install --editable '.[dev,optional]' --resolution=${{ matrix.config.resolution }}
uv pip install --editable '.[${{ matrix.config.extras }}]' --resolution=${{ matrix.config.resolution }}
- name: pytest split ${{ matrix.split }}
run: |
Expand Down
2 changes: 1 addition & 1 deletion dev_scripts/update_pt_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ def parse_ionic_radii():
ionic_radii[int(header[tok_idx])] = float(match.group(1))

if el in data:
data[el]["Ionic_radii" + suffix] = ionic_radii
data[el][f"Ionic_radii{suffix}"] = ionic_radii
if suffix == "_hs":
data[el]["Ionic_radii"] = ionic_radii
else:
Expand Down
2 changes: 1 addition & 1 deletion docs/apidoc/conf.py

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pymatgen/analysis/magnetism/analyzer.py
Original file line number Diff line number Diff line change
Expand Up @@ -614,7 +614,7 @@ def __init__(
Args:
structure: input structure
default_magmoms: (optional, defaults provided) dict of
magnetic elements to their initial magnetic moments in µB, generally
magnetic elements to their initial magnetic moments in μB, generally
these are chosen to be high-spin since they can relax to a low-spin
configuration during a DFT electronic configuration
strategies: different ordering strategies to use, choose from:
Expand Down
2 changes: 1 addition & 1 deletion pymatgen/analysis/phase_diagram.py
Original file line number Diff line number Diff line change
Expand Up @@ -2277,7 +2277,7 @@ def plot_element_profile(self, element, comp, show_label_index=None, xlim=5):
X value is the negative value of the chemical potential reference to
elemental chemical potential. For example, if choose Element("Li"),
X= -(µLi-µLi0), which corresponds to the voltage versus metal anode.
X= -(μLi-μLi0), which corresponds to the voltage versus metal anode.
Y values represent for the number of element uptake in this composition
(unit: per atom). All reactions are printed to help choosing the
profile steps you want to show label in the plot.
Expand Down
2 changes: 1 addition & 1 deletion pymatgen/command_line/gulp_caller.py
Original file line number Diff line number Diff line change
Expand Up @@ -764,7 +764,7 @@ def __init__(self, msg):
self.msg = msg

def __str__(self):
return "GulpError : " + self.msg
return f"GulpError : {self.msg}"


class GulpConvergenceError(Exception):
Expand Down
2 changes: 1 addition & 1 deletion pymatgen/core/periodic_table.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ def __init__(self, symbol: SpeciesLike) -> None:
Data is obtained from http://wikipedia.org/wiki/Atomic_radii_of_the_elements_(data_page).
van_der_waals_radius (float): Van der Waals radius for the element. This is the empirical value determined
from critical reviews of X-ray diffraction, gas kinetic collision cross-section, and other experimental
data by Bondi and later workers. The uncertainty in these values is on the order of 0.1 .
data by Bondi and later workers. The uncertainty in these values is on the order of 0.1 Å.
Data are obtained from "Atomic Radii of the Elements" in CRC Handbook of Chemistry and Physics,
91st Ed.; Haynes, W.M., Ed.; CRC Press: Boca Raton, FL, 2010.
mendeleev_no (int): Mendeleev number from definition given by Pettifor, D. G. (1984). A chemical scale
Expand Down
2 changes: 1 addition & 1 deletion pymatgen/core/trajectory.py
Original file line number Diff line number Diff line change
Expand Up @@ -569,7 +569,7 @@ def from_file(cls, filename: str | Path, constant_lattice: bool = True, **kwargs
is_mol = True

except ImportError as exc:
raise exc
raise ImportError("ASE is required to read .traj files. pip install ase") from exc

else:
supported_file_types = ("XDATCAR", "vasprun.xml", "*.traj")
Expand Down
5 changes: 1 addition & 4 deletions pymatgen/entries/computed_entries.py
Original file line number Diff line number Diff line change
Expand Up @@ -470,11 +470,8 @@ def __eq__(self, other: object) -> bool:
if not math.isclose(self.energy, other.energy):
return False

if self.composition != other.composition:
return False

# assumes that data, parameters are equivalent
return True
return self.composition == other.composition

@classmethod
def from_dict(cls, dct: dict) -> Self:
Expand Down
4 changes: 2 additions & 2 deletions pymatgen/io/abinit/pseudos.py
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ def to_str(self, verbose=0) -> str:
lines: list[str] = []
lines += (
f"<{type(self).__name__}: {self.basename}>",
" summary: " + self.summary.strip(),
f" summary: {self.summary.strip()}",
f" number of valence electrons: {self.Z_val}",
f" maximum angular momentum: {l2str(self.l_max)}",
f" angular momentum for local part: {l2str(self.l_local)}",
Expand Down Expand Up @@ -1512,7 +1512,7 @@ def plot_projectors(self, ax: plt.Axes = None, fontsize=12, **kwargs):
# ax.annotate("$r_c$", xy=(self.paw_radius + 0.1, 0.1))

# for state, rfunc in self.potentials.items():
# ax.plot(rfunc.mesh, rfunc.values, label="TPROJ: " + state)
# ax.plot(rfunc.mesh, rfunc.values, label=f"TPROJ: {state}")

# ax.legend(loc="best")

Expand Down
2 changes: 1 addition & 1 deletion pymatgen/io/abinit/variable.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ def __str__(self):

# Add units
if self.units:
line += " " + self.units
line += f" {self.units}"

return line

Expand Down
4 changes: 2 additions & 2 deletions pymatgen/io/ase.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,8 @@ def get_atoms(structure: SiteCollection, msonable: bool = True, **kwargs) -> MSO
if msonable:
atoms = MSONAtoms(atoms)

if "tags" in structure.site_properties:
atoms.set_tags(structure.site_properties["tags"])
if tags := structure.site_properties.get("tags"):
atoms.set_tags(tags)

# Set the site magmoms in the ASE Atoms object
# Note: ASE distinguishes between initial and converged
Expand Down
2 changes: 1 addition & 1 deletion pymatgen/io/cif.py
Original file line number Diff line number Diff line change
Expand Up @@ -1073,7 +1073,7 @@ def get_matching_coord(

if any(occu > 1 for occu in _sum_occupancies):
msg = (
f"Some occupancies ({list(filter(lambda x: x>1, _sum_occupancies))}) sum to > 1! If they are within "
f"Some occupancies ({list(filter(lambda x: x > 1, _sum_occupancies))}) sum to > 1! If they are within "
"the occupancy_tolerance, they will be rescaled. "
f"The current occupancy_tolerance is set to: {self._occupancy_tolerance}"
)
Expand Down
2 changes: 1 addition & 1 deletion pymatgen/io/lammps/inputs.py
Original file line number Diff line number Diff line change
Expand Up @@ -504,7 +504,7 @@ def get_str(self, ignore_comments: bool = False, keep_stages: bool = True) -> st
# Print first the name of the stage in a comment.
# We print this even if ignore_comments is True.
if "Comment" not in stage["stage_name"] and len(self.stages) > 1:
lammps_input += "\n# " + stage["stage_name"] + "\n"
lammps_input += f"\n# {stage['stage_name']}\n"

# In case of a block of comment, the header is not printed (useless)
else:
Expand Down
2 changes: 1 addition & 1 deletion pymatgen/io/nwchem.py
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,7 @@ def molecule(self):
def __str__(self):
out = []
if self.memory_options:
out.append("memory " + self.memory_options)
out.append(f"memory {self.memory_options}")
for d in self.directives:
out.append(f"{d[0]} {d[1]}")
out.append("geometry " + " ".join(self.geometry_options))
Expand Down
8 changes: 4 additions & 4 deletions pymatgen/symmetry/analyzer.py
Original file line number Diff line number Diff line change
Expand Up @@ -1554,8 +1554,8 @@ def generate_full_symmops(symmops: Sequence[SymmOp], tol: float) -> Sequence[Sym
# Uses an algorithm described in:
# Gregory Butler. Fundamental Algorithms for Permutation Groups.
# Lecture Notes in Computer Science (Book 559). Springer, 1991. page 15
UNIT = np.eye(4)
generators = [op.affine_matrix for op in symmops if not np.allclose(op.affine_matrix, UNIT)]
identity = np.eye(4)
generators = [op.affine_matrix for op in symmops if not np.allclose(op.affine_matrix, identity)]
if not generators:
# C1 symmetry breaks assumptions in the algorithm afterwards
return symmops
Expand All @@ -1574,9 +1574,9 @@ def generate_full_symmops(symmops: Sequence[SymmOp], tol: float) -> Sequence[Sym
" and rerun with a different tolerance."
)

d = np.abs(full - UNIT) < tol
d = np.abs(full - identity) < tol
if not np.any(np.all(np.all(d, axis=2), axis=1)):
full.append(UNIT)
full.append(identity)
return [SymmOp(op) for op in full]


Expand Down
8 changes: 6 additions & 2 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,11 @@
"pytest-split>=0.8",
"pytest>8",
"ruff>=0.4",
"typing-extensions>=4",
],
"ci": [
"pytest>=8",
"pytest-cov>=4",
"pytest-split>=0.8",
],
"docs": [
"sphinx",
Expand All @@ -83,7 +87,7 @@
"matgl>=1.1.1",
"netCDF4>=1.6.5",
"phonopy>=2.23",
"seekpath>=1.9.4",
"seekpath>=2.0.1",
# don't depend on tblite above 3.11 since unsupported https://github.com/tblite/tblite/issues/175
"tblite[ase]>=0.3.0; platform_system=='Linux' and python_version<'3.12'",
# "hiphive>=0.6",
Expand Down
16 changes: 10 additions & 6 deletions tests/core/test_trajectory.py
Original file line number Diff line number Diff line change
Expand Up @@ -474,14 +474,18 @@ def test_xdatcar_write(self):
self._check_traj_equality(self.traj, written_traj)

def test_from_file(self):
traj = Trajectory.from_file(f"{TEST_DIR}/LiMnO2_chgnet_relax.traj")
assert isinstance(traj, Trajectory)
try:
traj = Trajectory.from_file(f"{TEST_DIR}/LiMnO2_chgnet_relax.traj")
assert isinstance(traj, Trajectory)

# Check length of the trajectory
assert len(traj) == 2
# Check length of the trajectory
assert len(traj) == 2

# Check composition of the first frame of the trajectory
assert traj[0].formula == "Li2 Mn2 O4"
# Check composition of the first frame of the trajectory
assert traj[0].formula == "Li2 Mn2 O4"
except ImportError:
with pytest.raises(ImportError, match="ASE is required to read .traj files. pip install ase"):
Trajectory.from_file(f"{TEST_DIR}/LiMnO2_chgnet_relax.traj")

def test_index_error(self):
with pytest.raises(IndexError, match="index=100 out of range, trajectory only has 100 frames"):
Expand Down
13 changes: 13 additions & 0 deletions tests/io/pwmat/test_inputs.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ def test_write_file(self):

class TestGenKpt(PymatgenTest):
def test_from_structure(self):
pytest.importorskip("seekpath")
filepath = f"{TEST_DIR}/atom.config"
structure = Structure.from_file(filepath)
gen_kpt = GenKpt.from_structure(structure, dim=2, density=0.01)
Expand All @@ -88,6 +89,7 @@ def test_from_structure(self):
assert gen_kpt.kpath["path"] == [["GAMMA", "M", "K", "GAMMA"]]

def test_write_file(self):
pytest.importorskip("seekpath")
filepath = f"{TEST_DIR}/atom.config"
structure = Structure.from_file(filepath)
dim = 2
Expand All @@ -103,6 +105,7 @@ def test_write_file(self):

class TestHighSymmetryPoint(PymatgenTest):
def test_from_structure(self):
pytest.importorskip("seekpath")
filepath = f"{TEST_DIR}/atom.config"
structure = Structure.from_file(filepath)
high_symmetry_points = HighSymmetryPoint.from_structure(structure, dim=2, density=0.01)
Expand All @@ -112,6 +115,7 @@ def test_from_structure(self):
assert high_symmetry_points.reciprocal_lattice.shape == (3, 3)

def test_write_file(self):
pytest.importorskip("seekpath")
filepath = f"{TEST_DIR}/atom.config"
structure = Structure.from_file(filepath)
dim = 2
Expand All @@ -123,3 +127,12 @@ def test_write_file(self):
with zopen(tmp_filepath, "rt") as file:
tmp_high_symmetry_points_str = file.read()
assert tmp_high_symmetry_points_str == high_symmetry_points.get_str()


# simulate and test error message when seekpath is not installed
def test_err_msg_on_seekpath_not_installed(monkeypatch):
try:
import seekpath # noqa: F401
except ImportError:
with pytest.raises(RuntimeError, match="SeeK-path needs to be installed to use the convention of Hinuma et al"):
GenKpt.from_structure(Structure.from_file(f"{TEST_DIR}/atom.config"), dim=2, density=0.01)
2 changes: 1 addition & 1 deletion tests/io/test_ase.py
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,6 @@ def test_no_ase_err():

import pymatgen.io.ase

expected_msg = str(pymatgen.io.ase.no_ase_err)
expected_msg = str(pymatgen.io.ase.NO_ASE_ERR)
with pytest.raises(PackageNotFoundError, match=expected_msg):
pymatgen.io.ase.MSONAtoms()

0 comments on commit d1fac20

Please sign in to comment.