Skip to content

Commit

Permalink
Improve type annotations and comments for io.cif (#3820)
Browse files Browse the repository at this point in the history
* remove duplicate warning

* type and docstring tweaks

* add some types for io.cif

* add comments and types

* temp save of some formatting

* revert functional change

* revert unrelated changes

* fix unit test

* remove update that does nothing

* relocate `sub_space_group` and `space_groups` to their usage

* add more types

* pre-commit auto-fixes

* breaking: make parse_magmom/oxi_stats private

* remove merge header

* fix unit test

* add more types

* fix mypy errors

* add a few spaces

* remove if name ==main

* simplify "check to see if" in comments

* final tweaks

* revise docstring

* replace deprecated abc.abstractproperty

* add missing doc strings and standardize existing

* breaking: fix typo in method name orthongonalize_structure

* revert accidental change in test_composition.py

---------

Co-authored-by: Janosh Riebesell <janosh.riebesell@gmail.com>
  • Loading branch information
DanielYang59 and janosh authored May 15, 2024
1 parent 50043c9 commit bb68c78
Show file tree
Hide file tree
Showing 33 changed files with 606 additions and 507 deletions.
2 changes: 1 addition & 1 deletion docs/pymatgen.md

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

3 changes: 1 addition & 2 deletions pymatgen/alchemy/materials.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,7 @@


class TransformedStructure(MSONable):
"""Container object for new structures that include history of
transformations.
"""Container for new structures that include history of transformations.
Each transformed structure is made up of a sequence of structures with
associated transformation history.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1265,7 +1265,7 @@ def from_dict(cls, dct: dict) -> Self:


class NbSetWeight(MSONable, abc.ABC):
"""Abstract object for neighbors sets weights estimations."""
"""Abstract base class for neighbor set weight estimations."""

@abc.abstractmethod
def as_dict(self):
Expand All @@ -1282,7 +1282,7 @@ def weight(self, nb_set, structure_environments, cn_map=None, additional_info=No
additional_info: Additional information.
Returns:
Weight of the neighbors set.
float: Weight of the neighbors set.
"""


Expand Down Expand Up @@ -1313,7 +1313,7 @@ def weight(self, nb_set, structure_environments, cn_map=None, additional_info=No
additional_info: Additional information.
Returns:
Weight of the neighbors set.
float: Weight of the neighbors set.
"""
return self.aw(nb_set=nb_set)

Expand Down Expand Up @@ -1534,7 +1534,7 @@ def weight(self, nb_set, structure_environments, cn_map=None, additional_info=No
additional_info: Additional information.
Returns:
Weight of the neighbors set.
float: Weight of the neighbors set.
"""
fda_list = self.fda(nb_set=nb_set)
return self.eval(fda_list=fda_list)
Expand Down Expand Up @@ -1680,7 +1680,7 @@ def weight(self, nb_set, structure_environments, cn_map=None, additional_info=No
additional_info: Additional information.
Returns:
Weight of the neighbors set.
float: Weight of the neighbors set.
"""
effective_csm = get_effective_csm(
nb_set=nb_set,
Expand Down Expand Up @@ -1791,7 +1791,7 @@ def weight(self, nb_set, structure_environments, cn_map=None, additional_info=No
additional_info: Additional information.
Returns:
Weight of the neighbors set.
float: Weight of the neighbors set.
"""
effcsm = get_effective_csm(
nb_set=nb_set,
Expand Down Expand Up @@ -2006,7 +2006,7 @@ def weight(self, nb_set, structure_environments, cn_map=None, additional_info=No
additional_info: Additional information.
Returns:
Weight of the neighbors set.
float: Weight of the neighbors set.
"""
return self.cn_weights[len(nb_set)]

Expand Down Expand Up @@ -2186,7 +2186,7 @@ def weight(self, nb_set, structure_environments, cn_map=None, additional_info=No
additional_info: Additional information.
Returns:
Weight of the neighbors set.
float: Weight of the neighbors set.
"""
return self.area_weight(
nb_set=nb_set,
Expand Down Expand Up @@ -2379,7 +2379,7 @@ def weight(self, nb_set, structure_environments, cn_map=None, additional_info=No
additional_info: Additional information.
Returns:
Weight of the neighbors set.
float: Weight of the neighbors set.
"""
return self.weight_rf.eval(nb_set.distance_plateau())

Expand Down Expand Up @@ -2446,7 +2446,7 @@ def weight(self, nb_set, structure_environments, cn_map=None, additional_info=No
additional_info: Additional information.
Returns:
Weight of the neighbors set.
float: Weight of the neighbors set.
"""
return self.weight_rf.eval(nb_set.angle_plateau())

Expand Down Expand Up @@ -2509,7 +2509,7 @@ def weight(self, nb_set, structure_environments, cn_map=None, additional_info=No
additional_info: Additional information.
Returns:
Weight of the neighbors set.
float: Weight of the neighbors set.
"""
cn = cn_map[0]
isite = nb_set.isite
Expand Down Expand Up @@ -2590,7 +2590,7 @@ def weight(self, nb_set, structure_environments, cn_map=None, additional_info=No
additional_info: Additional information.
Returns:
Weight of the neighbors set.
float: Weight of the neighbors set.
"""
cn = cn_map[0]
isite = nb_set.isite
Expand Down
2 changes: 1 addition & 1 deletion pymatgen/analysis/cost.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ def get_entries(self, chemsys):

@singleton
class CostDBElements(CostDBCSV):
"""Singleton object that provides the cost data for elements."""
"""Singleton that provides the cost data for elements."""

def __init__(self):
CostDBCSV.__init__(self, f"{module_dir}/costdb_elements.csv")
Expand Down
2 changes: 1 addition & 1 deletion pymatgen/analysis/ewald.py
Original file line number Diff line number Diff line change
Expand Up @@ -638,7 +638,7 @@ def _recurse(self, matrix, m_list, indices, output_m_list=None):
indices: Set of indices which haven't had a permutation
performed on them.
"""
# check to see if we've found all the solutions that we need
# Check if we've found all the solutions that we need
if self._finished:
return

Expand Down
2 changes: 1 addition & 1 deletion pymatgen/analysis/fragmenter.py
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ def _fragment_one_level(self, old_frag_dict: dict) -> dict:
"""
Perform one step of iterative fragmentation on a list of molecule graphs. Loop through the graphs,
then loop through each graph's edges and attempt to remove that edge in order to obtain two
disconnected subgraphs, aka two new fragments. If successful, check to see if the new fragments
disconnected subgraphs, aka two new fragments. If successful, check if the new fragments
are already present in self.unique_fragments, and append them if not. If unsuccessful, we know
that edge belongs to a ring. If we are opening rings, do so with that bond, and then again
check if the resulting fragment is present in self.unique_fragments and add it if it is not.
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 @@ -161,7 +161,7 @@ def __init__(
except ValueError:
warnings.warn(f"Could not assign valences for {structure.reduced_formula}")

# check to see if structure has magnetic moments
# Check if structure has magnetic moments
# on site properties or species spin properties,
# prioritize site properties

Expand Down
2 changes: 1 addition & 1 deletion pymatgen/analysis/reaction_calculator.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@


class BalancedReaction(MSONable):
"""An object representing a complete chemical reaction."""
"""Represent a complete chemical reaction."""

# Tolerance for determining if a particular component fraction is > 0.
TOLERANCE = 1e-6
Expand Down
2 changes: 1 addition & 1 deletion pymatgen/analysis/thermochemistry.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@


class ThermoData:
"""A object container for an experimental Thermochemical Data."""
"""Container for experimental thermo-chemical data."""

def __init__(
self,
Expand Down
5 changes: 0 additions & 5 deletions pymatgen/core/libxcfunc.py
Original file line number Diff line number Diff line change
Expand Up @@ -499,8 +499,3 @@ def from_dict(cls, dct: dict) -> Self:
def to_json(self) -> str:
"""Get a json string representation of the LibxcFunc."""
return json.dumps(self.as_dict(), cls=MontyEncoder)


if __name__ == "__main__":
for xc in LibxcFunc:
print(xc)
4 changes: 2 additions & 2 deletions pymatgen/core/structure.py
Original file line number Diff line number Diff line change
Expand Up @@ -4056,7 +4056,7 @@ def substitute(self, index: int, func_group: IMolecule | Molecule | str, bond_or
fgroup = func_group

else:
# Check to see whether the functional group is in database.
# Check whether the functional group is in database.
if func_group not in FunctionalGroups:
raise ValueError(
f"Can't find functional group {func_group!r} in list. Provide explicit coordinates instead"
Expand Down Expand Up @@ -4949,7 +4949,7 @@ def substitute(self, index: int, func_group: IMolecule | Molecule | str, bond_or
if isinstance(func_group, Molecule):
functional_group = func_group
else:
# Check to see whether the functional group is in database.
# Check whether the functional group is in database.
if func_group not in FunctionalGroups:
raise RuntimeError("Can't find functional group in list. Provide explicit coordinate instead")
functional_group = FunctionalGroups[func_group]
Expand Down
4 changes: 2 additions & 2 deletions pymatgen/core/xcfunc.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@


class XcFunc(MSONable):
"""This object stores information about the XC correlation functional.
"""Store information about the XC correlation functional.
Client code usually creates the object by calling the class methods:
Expand Down Expand Up @@ -162,7 +162,7 @@ def aliases(cls) -> list[str]:

@classmethod
def asxc(cls, obj) -> Self:
"""Convert object into XcFunc."""
"""Convert to XcFunc."""
if isinstance(obj, cls):
return obj
if isinstance(obj, str):
Expand Down
5 changes: 2 additions & 3 deletions pymatgen/electronic_structure/bandstructure.py
Original file line number Diff line number Diff line change
Expand Up @@ -681,9 +681,8 @@ def from_old_dict(cls, dct) -> Self:


class BandStructureSymmLine(BandStructure, MSONable):
r"""This object stores band structures along selected (symmetry) lines in the
Brillouin zone. We call the different symmetry lines (ex: \\Gamma to Z)
"branches".
r"""Store band structures along selected (symmetry) lines in the Brillouin zone.
We call the different symmetry lines (ex: \\Gamma to Z) "branches".
"""

def __init__(
Expand Down
2 changes: 1 addition & 1 deletion pymatgen/electronic_structure/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,7 @@ def get_suggested_saxis(magmoms):

@staticmethod
def are_collinear(magmoms) -> bool:
"""Check to see if a set of magnetic moments are collinear with each other.
"""Check if a set of magnetic moments are collinear with each other.
Args:
magmoms: list of magmoms (Magmoms, scalars or vectors).
Expand Down
2 changes: 1 addition & 1 deletion pymatgen/entries/compatibility.py
Original file line number Diff line number Diff line change
Expand Up @@ -1354,7 +1354,7 @@ def get_adjustments(self, entry: ComputedEntry) -> list[EnergyAdjustment]:
# TODO - detection of embedded water molecules is not very sophisticated
# Should be replaced with some kind of actual structure detection

# For any compound except water, check to see if it is a hydrate (contains
# For any compound except water, check if it is a hydrate (contains
# H2O in its structure). If so, adjust the energy to remove MU_H2O eV per
# embedded water molecule.
# in other words, we assume that the DFT energy of such a compound is really
Expand Down
Loading

0 comments on commit bb68c78

Please sign in to comment.