From f79dcd1f5fe996e9689fcf3d10fcf74fb392c93a Mon Sep 17 00:00:00 2001 From: benrich37 <76569522+benrich37@users.noreply.github.com> Date: Wed, 2 Oct 2024 20:38:14 -0600 Subject: [PATCH 1/2] Adding edit to the "valence" property as described in review comment + update to existing code so that pre-commit will let me commit --- src/pymatgen/core/periodic_table.py | 80 ++++++++++++++++++----------- 1 file changed, 51 insertions(+), 29 deletions(-) diff --git a/src/pymatgen/core/periodic_table.py b/src/pymatgen/core/periodic_table.py index 63419d5f986..ea5e01de72e 100644 --- a/src/pymatgen/core/periodic_table.py +++ b/src/pymatgen/core/periodic_table.py @@ -20,6 +20,7 @@ from pymatgen.core.units import SUPPORTED_UNIT_NAMES, FloatWithUnit, Ha_to_eV, Length, Mass, Unit from pymatgen.io.core import ParseError from pymatgen.util.string import Stringify, formula_double_format +from pymatgen.util.typing import SpeciesLike if TYPE_CHECKING: from collections.abc import Callable @@ -27,7 +28,6 @@ from typing_extensions import Self - from pymatgen.util.typing import SpeciesLike # Load element data from JSON file with open(Path(__file__).absolute().parent / "periodic_table.json", encoding="utf-8") as ptable_json: @@ -204,7 +204,8 @@ def __getattr__(self, item: str) -> Any: if val is None or str(val).startswith("no data"): warnings.warn(f"No data available for {item} for {self.symbol}") val = None - elif isinstance(val, list | dict): + # elif isinstance(val, dict | list): + elif type(val) in [list, dict]: # pre-commit fix pass else: try: @@ -475,28 +476,35 @@ def n_electrons(self) -> int: return sum(t[-1] for t in self.full_electronic_structure) @property - def valence(self) -> tuple[int | np.nan, int]: + def valences(self) -> list[tuple[int | np.nan, int]]: """Valence subshell angular moment (L) and number of valence e- (v_e), obtained from full electron config, where L=0, 1, 2, or 3 for s, p, d, and f orbitals, respectively. """ if self.group == 18: - return np.nan, 0 # The number of valence of noble gas is 0 + return [(np.nan, 0)] # The number of valence of noble gas is 0 L_symbols = "SPDFGHIKLMNOQRTUVWXYZ" - valence: list[tuple[int, int]] = [] + valences: list[tuple[int | np.nan, int]] = [] full_electron_config = self.full_electronic_structure last_orbital = full_electron_config[-1] for n, l_symbol, ne in full_electron_config: idx = L_symbols.lower().index(l_symbol) if ne < (2 * idx + 1) * 2 or ( - (n, l_symbol, ne) == last_orbital and ne == (2 * idx + 1) * 2 and len(valence) == 0 + (n, l_symbol, ne) == last_orbital and ne == (2 * idx + 1) * 2 and len(valences) == 0 ): # check for full last shell (e.g. column 2) - valence.append((idx, ne)) - if len(valence) > 1: - raise ValueError(f"{self} has ambiguous valence") + valences.append((idx, ne)) + return valences - return valence[0] + @property + def valence(self) -> tuple[int | np.nan, int]: + """Valence subshell angular moment (L) and number of valence e- (v_e), + obtained from full electron config, where L=0, 1, 2, or 3 for s, p, d, + and f orbitals, respectively. + """ + if len(self.valences) > 1: + raise ValueError(f"{self} has ambiguous valence") + return self.valences[0] @property def term_symbols(self) -> list[list[str]]: @@ -523,7 +531,8 @@ def term_symbols(self) -> list[list[str]]: # Total ML = sum(ml1, ml2), Total MS = sum(ms1, ms2) TL = [sum(ml_ms[comb[e]][0] for e in range(v_e)) for comb in e_config_combs] TS = [sum(ml_ms[comb[e]][1] for e in range(v_e)) for comb in e_config_combs] - comb_counter = Counter(zip(TL, TS, strict=True)) + # comb_counter: Counter = Counter(zip(TL, TS, strict=True)) + comb_counter: Counter = Counter([(TL[i], TS[i]) for i in range(len(TL))]) # pre-commit edit term_symbols = [] L_symbols = "SPDFGHIKLMNOQRTUVWXYZ" @@ -1180,29 +1189,40 @@ def n_electrons(self) -> int: # NOTE - copied exactly from Element. Refactoring / inheritance may improve # robustness + @property + def valences(self) -> list[tuple[int | np.nan, int]]: + """List of valence subshell angular moment (L) and number of valence e- (v_e), + obtained from full electron config, where L=0, 1, 2, or 3 for s, p, d, + and f orbitals, respectively. + + + """ + return self.element.valences + # if self.group == 18: + # return [(np.nan, 0)] # The number of valence of noble gas is 0 + + # L_symbols = "SPDFGHIKLMNOQRTUVWXYZ" + # valences: list[tuple[int, int]] = [] + # full_electron_config = self.full_electronic_structure + # last_orbital = full_electron_config[-1] + # for n, l_symbol, ne in full_electron_config: + # idx = L_symbols.lower().index(l_symbol) + # if ne < (2 * idx + 1) * 2 or ( + # (n, l_symbol, ne) == last_orbital and ne == (2 * idx + 1) * 2 and len(valences) == 0 + # ): # check for full last shell (e.g. column 2) + # valences.append((idx, ne)) + # return valences + @property def valence(self) -> tuple[int | np.nan, int]: """Valence subshell angular moment (L) and number of valence e- (v_e), obtained from full electron config, where L=0, 1, 2, or 3 for s, p, d, and f orbitals, respectively. """ - if self.group == 18: - return np.nan, 0 # The number of valence of noble gas is 0 - - L_symbols = "SPDFGHIKLMNOQRTUVWXYZ" - valence: list[tuple[int, int]] = [] - full_electron_config = self.full_electronic_structure - last_orbital = full_electron_config[-1] - for n, l_symbol, ne in full_electron_config: - idx = L_symbols.lower().index(l_symbol) - if ne < (2 * idx + 1) * 2 or ( - (n, l_symbol, ne) == last_orbital and ne == (2 * idx + 1) * 2 and len(valence) == 0 - ): # check for full last shell (e.g. column 2) - valence.append((idx, ne)) - if len(valence) > 1: - raise ValueError(f"{self} has ambiguous valence") - - return valence[0] + return self.element.valence + # if len(self.valences) > 1: + # raise ValueError(f"{self} has ambiguous valence") + # return self.valences[0] @property def ionic_radius(self) -> float | None: @@ -1628,7 +1648,9 @@ def get_el_sp(obj: int | SpeciesLike) -> Element | Species | DummySpecies: of properties that can be determined. """ # If obj is already an Element or Species, return as is - if isinstance(obj, Element | Species | DummySpecies): + # Roundabout way to check if obj is Element | Soecies | DummySpecies without angering the mypy bug + if isinstance(obj, SpeciesLike) and not isinstance(obj, str): + # if isinstance(obj, Element | Species | DummySpecies): if getattr(obj, "_is_named_isotope", None): return Element(obj.name) if isinstance(obj, Element) else Species(str(obj)) return obj From 359261d2dbaf446deb0c386b10490e3d1acd51f4 Mon Sep 17 00:00:00 2001 From: benrich37 <76569522+benrich37@users.noreply.github.com> Date: Tue, 22 Oct 2024 14:36:21 -0600 Subject: [PATCH 2/2] Replacing "get_Atom_valence_electrons" function with newly added "Valences" method in pmg Element class --- src/pymatgen/core/periodic_table.py | 25 +++++++++++++++++----- src/pymatgen/io/jdftx/jdftxoutfileslice.py | 6 +++--- 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/src/pymatgen/core/periodic_table.py b/src/pymatgen/core/periodic_table.py index 7d212cc1ff8..7d33177aba0 100644 --- a/src/pymatgen/core/periodic_table.py +++ b/src/pymatgen/core/periodic_table.py @@ -20,7 +20,6 @@ from pymatgen.core.units import SUPPORTED_UNIT_NAMES, FloatWithUnit, Ha_to_eV, Length, Mass, Unit from pymatgen.io.core import ParseError from pymatgen.util.string import Stringify, formula_double_format -from pymatgen.util.typing import SpeciesLike if TYPE_CHECKING: from collections.abc import Callable @@ -28,6 +27,8 @@ from typing_extensions import Self + from pymatgen.util.typing import SpeciesLike + # Load element data from JSON file with open(Path(__file__).absolute().parent / "periodic_table.json", encoding="utf-8") as ptable_json: @@ -1651,12 +1652,26 @@ def get_el_sp(obj: int | SpeciesLike) -> Element | Species | DummySpecies: of properties that can be determined. """ # If obj is already an Element or Species, return as is - # Roundabout way to check if obj is Element | Soecies | DummySpecies without angering the mypy bug - if isinstance(obj, SpeciesLike) and not isinstance(obj, str): - # if isinstance(obj, Element | Species | DummySpecies): + # Note: the below three if statements are functionally equivalent to the commented out + # code. They only exist due to a bug in mypy that doesn't allow the commented out code. + # This should be fixed once mypy fixes this bug. + if isinstance(obj, Element): + if getattr(obj, "_is_named_isotope", None): + return Element(obj.name) + return obj + if isinstance(obj, Species): + if getattr(obj, "_is_named_isotope", None): + return Species(str(obj)) + return obj + if isinstance(obj, Species): if getattr(obj, "_is_named_isotope", None): - return Element(obj.name) if isinstance(obj, Element) else Species(str(obj)) + return Species(str(obj)) return obj + # if isinstance(obj, Element | Species | DummySpecies): + # if type(obj) in [Element, Species, DummySpecies]: + # if getattr(obj, "_is_named_isotope", None): + # return Element(obj.name) if isinstance(obj, Element) else Species(str(obj)) + # return obj # If obj is an integer, return the Element with atomic number obj try: diff --git a/src/pymatgen/io/jdftx/jdftxoutfileslice.py b/src/pymatgen/io/jdftx/jdftxoutfileslice.py index 99e14065a62..a93cc4faeb1 100644 --- a/src/pymatgen/io/jdftx/jdftxoutfileslice.py +++ b/src/pymatgen/io/jdftx/jdftxoutfileslice.py @@ -19,7 +19,6 @@ from pymatgen.core.periodic_table import Element from pymatgen.core.trajectory import Trajectory from pymatgen.core.units import Ha_to_eV, ang_to_bohr -from pymatgen.io.jdftx.data import get_atom_valence_electrons from pymatgen.io.jdftx.jminsettings import ( JMinSettings, JMinSettingsElectronic, @@ -860,8 +859,9 @@ def set_pseudo_vars_t1(self, text: list[str]) -> None: # total_elec_dict = dict(zip(self.atom_types, atom_total_elec, strict=False)) # Explicit zipping due to pre-commit in three lines below element_total_electrons = np.array([total_elec_dict[x] for x in self.atom_elements]) - element_valence_electrons = np.array([get_atom_valence_electrons(x) for x in self.atom_elements]) - # element_valence_electrons = np.array([Element(x).valence[1] for x in self.atom_elements]) + pmg_elements = [Element(x) for x in self.atom_elements] + # element_valence_electrons = np.array([get_atom_valence_electrons(x) for x in self.atom_elements]) + element_valence_electrons = np.array([np.sum(np.array([v[1] for v in el.valences])) for el in pmg_elements]) # element_valence_electrons = np.array([atom_valence_electrons[x] for x in self.atom_elements]) element_semicore_electrons = element_total_electrons - element_valence_electrons self.total_electrons_uncharged = np.sum(element_total_electrons)