Skip to content

Commit

Permalink
Bug fix: handle non-integer oxidation states in Species (#3868)
Browse files Browse the repository at this point in the history
* add fractional oxi state handling to species init and test cases

* pre-commit auto-fixes

* better error message on unparsable oxi states

---------

Co-authored-by: Janosh Riebesell <janosh.riebesell@gmail.com>
  • Loading branch information
esoteric-ephemera and janosh authored Jun 7, 2024
1 parent 7c3ef9e commit 14988c0
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 3 deletions.
8 changes: 6 additions & 2 deletions pymatgen/core/periodic_table.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
from monty.json import MSONable

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

if TYPE_CHECKING:
Expand Down Expand Up @@ -982,8 +983,11 @@ def __init__(
)
if isinstance(symbol, str) and symbol.endswith(("+", "-")):
# Extract oxidation state from symbol
symbol, oxi = re.match(r"([A-Za-z]+)([0-9]*[\+\-])", symbol).groups() # type: ignore[union-attr]
self._oxi_state: float | None = (1 if "+" in oxi else -1) * float(oxi[:-1] or 1)
try:
symbol, oxi = re.match(r"([A-Za-z]+)([0-9\.0-9]*[\+\-])", symbol).groups() # type: ignore[union-attr]
self._oxi_state: float | None = (1 if "+" in oxi else -1) * float(oxi[:-1] or 1)
except AttributeError:
raise ParseError(f"Failed to parse {symbol=}")
else:
self._oxi_state = oxidation_state

Expand Down
12 changes: 11 additions & 1 deletion tests/core/test_periodic_table.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import math
import pickle
import re
from copy import deepcopy
from enum import Enum

Expand All @@ -12,6 +13,7 @@
from pymatgen.core import DummySpecies, Element, Species, get_el_sp
from pymatgen.core.periodic_table import ElementBase, ElementType
from pymatgen.core.units import Ha_to_eV
from pymatgen.io.core import ParseError
from pymatgen.util.testing import PymatgenTest


Expand Down Expand Up @@ -529,12 +531,20 @@ def test_stringify(self):
("P5+", "P", 5),
("Na0+", "Na", 0),
("Na0-", "Na", 0),
("C0.53-", "C", -0.53),
("Tc3.498+", "Tc", 3.498),
],
)
def test_symbol_oxi_state_str(symbol_oxi, expected_element, expected_oxi_state):
species = Species(symbol_oxi)
assert species._el.symbol == expected_element
assert species._oxi_state == expected_oxi_state
assert species._oxi_state == pytest.approx(expected_oxi_state, rel=1.0e-6)


def test_symbol_oxi_state_str_raises():
symbol = "Fe2.5f2123+" # invalid oxidation state
with pytest.raises(ParseError, match=re.escape(f"Failed to parse {symbol=}")):
_ = Species(symbol)


class TestDummySpecies:
Expand Down

0 comments on commit 14988c0

Please sign in to comment.