Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Utility module updates #307

Merged
merged 15 commits into from
Aug 29, 2024
50 changes: 50 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
# Contributing

This is a quick guide on how to follow best practice and contribute smoothly to `SMACT`.

## Workflow

We follow the [GitHub flow](<https://docs.github.com/en/get-started/using-github/github-flow>), using
branches for new work and pull requests for verifying the work.

The steps for a new piece of work can be summarised as follows:

1. Push up or create [an issue](https://github.com/WMD-group/SMACT/issues).
2. Create a branch from main, with a sensible name that relates to the issue.
3. Do the work and commit changes to the branch. Push the branch
regularly to GitHub to make sure no work is accidentally lost.
4. Write or update unit tests for the code you work on.
5. When you are finished with the work, ensure that all of the unit
tests pass on your own machine.
6. Open a pull request [on the pull request page](https://github.com/WMD-group/SMACT/pulls).
7. If nobody acknowledges your pull request promptly, feel free to poke one of the main developers into action.

## Pull requests

For a general overview of using pull requests on GitHub look [in the GitHub docs](https://help.github.com/en/articles/about-pull-requests).

When creating a pull request you should:

- Ensure that the title succinctly describes the changes so it is easy to read on the overview page
- Reference the issue which the pull request is closing

Recommended reading: [How to Write the Perfect Pull Request](https://github.blog/2015-01-21-how-to-write-the-perfect-pull-request/)

## Dev requirements

When developing locally, it is recommended to install the python packages in `requirements-dev.txt`.

```bash
pip install -r requirements-dev.txt
```

This will allow you to run the tests locally with pytest as described in the main README,
as well as run pre-commit hooks to automatically format python files with isort and black.
To install the pre-commit hooks (only needs to be done once):

```bash
pre-commit install
pre-commit run --all-files # optionally run hooks on all files
```

Pre-commit hooks will check all files when you commit changes, automatically fixing any files which are not formatted correctly. Those files will need to be staged again before re-attempting the commit.
11 changes: 1 addition & 10 deletions smact/properties.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import numpy as np

import smact
from smact.utils.composition import parse_formula


def eneg_mulliken(element: Union[smact.Element, str]) -> float:
Expand Down Expand Up @@ -179,16 +180,6 @@ def valence_electron_count(compound: str) -> float:
Raises:
ValueError: If an element in the compound is not found in the valence data.
"""
from typing import Dict

def parse_formula(formula: str) -> Dict[str, int]:
pattern = re.compile(r"([A-Z][a-z]*)(\d*)")
elements = pattern.findall(formula)
element_stoich: Dict[str, int] = defaultdict(int)
for element, count in elements:
count = int(count) if count else 1
element_stoich[element] += count
return element_stoich

def get_element_valence(element: str) -> int:
try:
Expand Down
76 changes: 76 additions & 0 deletions smact/tests/test_utils.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
import unittest

from pymatgen.core import Composition

from smact import Element
from smact.screening import smact_filter
from smact.utils.composition import comp_maker, formula_maker, parse_formula


class TestComposition(unittest.TestCase):
"""Test composition utilities"""

def setUp(self) -> None:
self.mock_filter_output = [
(("Fe", "O"), (2, -2), (1, 1)),
(("Fe", "O"), (1, 1)),
(("Fe", "Fe", "O"), (2, 3, -2), (1, 2, 4)),
]
self.smact_filter_output = smact_filter(
els=[Element("Li"), Element("Ge"), Element("P"), Element("S")],
stoichs=[[10], [1], [2], [12]],
)

def test_parse_formula(self):
"""Test the parse_formula function"""

formulas = ["Li10GeP2S12", "Mg0.5O0.5", "CaMg(CO3)2"]

LGPS = parse_formula(formulas[0])
self.assertIsInstance(LGPS, dict)
for el_sym, ammt in LGPS.items():
self.assertIsInstance(el_sym, str)
self.assertIsInstance(ammt, float)
self.assertEqual(LGPS["Li"], 10)
self.assertEqual(LGPS["Ge"], 1)
self.assertEqual(LGPS["P"], 2)
self.assertEqual(LGPS["S"], 12)

MgO = parse_formula(formulas[1])
self.assertIsInstance(MgO, dict)
self.assertEqual(MgO["Mg"], 0.5)
self.assertEqual(MgO["O"], 0.5)

dolomite = parse_formula(formulas[2])
self.assertIsInstance(dolomite, dict)
self.assertEqual(dolomite["Ca"], 1)
self.assertEqual(dolomite["Mg"], 1)
self.assertEqual(dolomite["C"], 2)
self.assertEqual(dolomite["O"], 6)

def test_comp_maker(self):
"""Test the comp_maker function"""
comp1 = comp_maker(self.mock_filter_output[0])
comp2 = comp_maker(self.mock_filter_output[1])
comp3 = comp_maker(self.mock_filter_output[2])
comp4 = comp_maker(self.smact_filter_output[1])
for comp in [comp1, comp2, comp3, comp4]:
self.assertIsInstance(comp, Composition)
self.assertEqual(Composition("FeO"), comp2)
self.assertEqual(Composition({"Fe2+": 1, "O2-": 1}), comp1)
self.assertEqual(Composition({"Fe2+": 1, "Fe3+": 2, "O2-": 4}), comp3)
self.assertEqual(
Composition({"Li+": 10, "Ge4+": 1, "P5+": 2, "S2-": 12}), comp4
)

def test_formula_maker(self):
"""Test the formula_maker function"""
form1 = formula_maker(self.mock_filter_output[0])
form2 = formula_maker(self.mock_filter_output[1])
form3 = formula_maker(self.mock_filter_output[2])
form4 = formula_maker(self.smact_filter_output[1])
self.assertEqual(form1, "FeO")
self.assertEqual(form2, "FeO")
self.assertEqual(form1, form2)
self.assertEqual(form3, "Fe3O4")
self.assertEqual(form4, "Li10Ge(PS6)2")
95 changes: 95 additions & 0 deletions smact/utils/composition.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
"""Utility functioms for handling elements, species, formulas and composition"""
import re
from collections import defaultdict

from pymatgen.core import Composition

from smact.structure_prediction.utilities import unparse_spec


# Adapted from ElementEmbeddings and Pymatgen
def parse_formula(formula: str) -> dict[str, int]:
"""Parse a formula into a dict of el:amt

Args:
formula (str): Chemical formula

Returns:
dict: Dictionary of element symbol: amount
"""
regex = r"\(([^\(\)]+)\)\s*([\.e\d]*)"
r = re.compile(regex)
m = re.search(r, formula)
if m:
factor = 1.0
if m.group(2) != "":
factor = float(m.group(2))
unit_sym_dict = _get_sym_dict(m.group(1), factor)
expanded_sym = "".join(
[f"{el}{amt}" for el, amt in unit_sym_dict.items()]
)
expanded_formula = formula.replace(m.group(), expanded_sym)
return parse_formula(expanded_formula)
return _get_sym_dict(formula, 1)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct the return type hint.

The return type hint should be dict[str, float] instead of dict[str, int] to accommodate non-integer stoichiometries.

-def parse_formula(formula: str) -> dict[str, int]:
+def parse_formula(formula: str) -> dict[str, float]:
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def parse_formula(formula: str) -> dict[str, int]:
"""Parse a formula into a dict of el:amt
Args:
formula (str): Chemical formula
Returns:
dict: Dictionary of element symbol: amount
"""
regex = r"\(([^\(\)]+)\)\s*([\.e\d]*)"
r = re.compile(regex)
m = re.search(r, formula)
if m:
factor = 1.0
if m.group(2) != "":
factor = float(m.group(2))
unit_sym_dict = _get_sym_dict(m.group(1), factor)
expanded_sym = "".join(
[f"{el}{amt}" for el, amt in unit_sym_dict.items()]
)
expanded_formula = formula.replace(m.group(), expanded_sym)
return parse_formula(expanded_formula)
return _get_sym_dict(formula, 1)
def parse_formula(formula: str) -> dict[str, float]:
"""Parse a formula into a dict of el:amt
Args:
formula (str): Chemical formula
Returns:
dict: Dictionary of element symbol: amount
"""
regex = r"\(([^\(\)]+)\)\s*([\.e\d]*)"
r = re.compile(regex)
m = re.search(r, formula)
if m:
factor = 1.0
if m.group(2) != "":
factor = float(m.group(2))
unit_sym_dict = _get_sym_dict(m.group(1), factor)
expanded_sym = "".join(
[f"{el}{amt}" for el, amt in unit_sym_dict.items()]
)
expanded_formula = formula.replace(m.group(), expanded_sym)
return parse_formula(expanded_formula)
return _get_sym_dict(formula, 1)



def _get_sym_dict(formula: str, factor: float) -> dict[str, float]:
sym_dict: dict[str, float] = defaultdict(float)
regex = r"([A-Z][a-z]*)\s*([-*\.e\d]*)"
r = re.compile(regex)
for m in re.finditer(r, formula):
el = m.group(1)
amt = 1.0
if m.group(2).strip() != "":
amt = float(m.group(2))
sym_dict[el] += amt * factor
formula = formula.replace(m.group(), "", 1)
if formula.strip():
msg = f"{formula} is an invalid formula"
raise ValueError(msg)

Check warning on line 49 in smact/utils/composition.py

View check run for this annotation

Codecov / codecov/patch

smact/utils/composition.py#L48-L49

Added lines #L48 - L49 were not covered by tests
Comment on lines +38 to +51
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add tests for error handling.

The function raises a ValueError if the formula is invalid. Ensure that this error handling is covered by tests.

Do you want me to generate the unit testing code or open a GitHub issue to track this task?

Tools
GitHub Check: codecov/patch

[warning] 50-51: smact/utils/composition.py#L50-L51
Added lines #L50 - L51 were not covered by tests


return sym_dict


def comp_maker(
smact_filter_output: tuple[str, int, int] | tuple[str, int]
) -> Composition:
"""Convert an output of smact.screening.smact_filer into a Pymatgen Compositions

Args:
smact_filter_output (tuple[str, int, int]|tuple[str, int]): An item in the list returned from smact_filter

Returns:
composition (pymatgen.core.Composition): An instance of the Composition class
"""
if len(smact_filter_output) == 2:
form = []
for el, ammt in zip(smact_filter_output[0], smact_filter_output[-1]):
form.append(el)
form.append(ammt)
form = "".join(str(e) for e in form)
else:
form = {}
for el, ox, ammt in zip(
smact_filter_output[0],
smact_filter_output[1],
smact_filter_output[2],
):
sp = unparse_spec((el, ox))
form[sp] = ammt
return Composition(form)


def formula_maker(
smact_filter_output: tuple[str, int, int] | tuple[str, int]
) -> str:
"""Convert an output of smact.screening.smact_filter into a formula.

Args:
smact_filter_output (tuple[str, int, int]|tuple[str, int]): An item in the list returned from smact_filter

Returns:
formula (str): A formula

"""
return comp_maker(smact_filter_output).reduced_formula
Loading