Skip to content

Commit

Permalink
Fix _SimpleMolecule atom names in roundtrip (#1928)
Browse files Browse the repository at this point in the history
* Reproduce issue #1927

* Ensure atom name is stored in `_SimpleAtom` dicts

* Update release history
  • Loading branch information
mattwthompson authored Aug 16, 2024
1 parent 6a1c6a2 commit 1395a6b
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 8 deletions.
9 changes: 2 additions & 7 deletions docs/releasehistory.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,15 @@ Releases follow the `major.minor.micro` scheme recommended by [PEP440](https://w
* `minor` increments add features but do not break API compatibility
* `micro` increments represent bugfix releases or improvements in documentation

## Current development

### API-breaking changes

### Behavior changes
## 0.16.4

### Bugfixes

### New features
- [PR #1928](https://github.com/openforcefield/openff-toolkit/pull/1928): Fixes a bug in which atom names were lost when adding `_SimpleMolecule` to a topology or serializing through a dictionary.

### Improved documentation and warnings
- [PR #1925](https://github.com/openforcefield/openff-toolkit/pull/1925): Adds a FAQ entry describing how to send a prepared system to HPC resources.


## 0.16.3

### New features
Expand Down
22 changes: 21 additions & 1 deletion openff/toolkit/_tests/test_mm_molecule.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
dipeptide_residues_perceived as create_dipeptide,
)
from openff.toolkit._tests.utils import get_data_file_path, requires_rdkit
from openff.toolkit.topology._mm_molecule import _SimpleMolecule
from openff.toolkit.topology._mm_molecule import _SimpleAtom, _SimpleMolecule


@pytest.fixture()
Expand Down Expand Up @@ -246,6 +246,26 @@ def test_generate_unique_atom_names(self, water):
water.atom(1).name = "foo"
assert water.has_unique_atom_names is False

def test_atom_names_roundtrip(self, water):
water.atom(0).name = "FOO"
water.atom(1).name = "BAR"
water.atom(2).name = "BAZ"

roundtrip = copy.deepcopy(water)

assert roundtrip.atom(0).name == "FOO"
assert roundtrip.atom(1).name == "BAR"
assert roundtrip.atom(2).name == "BAZ"


class TestSimpleAtom:
def test_atom_name_in_dict(self):
atom = _SimpleAtom(atomic_number=7, name="FLAG")

assert atom.to_dict()["name"] == "FLAG"

assert copy.deepcopy(atom).name == "FLAG"


class TestImpropers:
@pytest.mark.parametrize(
Expand Down
16 changes: 16 additions & 0 deletions openff/toolkit/_tests/test_topology.py
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,22 @@ def test_from_molecule_nonlist(self):

topology.add_molecules("CC.CCO")

def test_add_simple_molecule_atom_names(self):
"""Reproduce issue #1927"""
simple = _SimpleMolecule.from_molecule(Molecule.from_smiles("C"))

for index, letter in enumerate("BLAHB"):
simple.atom(index).name = letter

topology = Topology()
topology.add_molecule(simple)

assert topology.atom(0).name == "B"
assert topology.atom(1).name == "L"
assert topology.atom(2).name == "A"
assert topology.atom(3).name == "H"
assert topology.atom(4).name == "B"

def test_reinitialization_box_vectors(self):
topology = Topology()
assert Topology(topology).box_vectors is None
Expand Down
2 changes: 2 additions & 0 deletions openff/toolkit/topology/_mm_molecule.py
Original file line number Diff line number Diff line change
Expand Up @@ -627,6 +627,7 @@ def to_dict(self) -> dict[str, Union[dict, str, int]]:
atom_dict: dict[str, Union[dict, str, int]] = dict()
atom_dict["metadata"] = dict(self.metadata)
atom_dict["atomic_number"] = self._atomic_number
atom_dict["name"] = self._name

keys_to_skip = ["metadata", "molecule", "bonds"]

Expand All @@ -636,6 +637,7 @@ def to_dict(self) -> dict[str, Union[dict, str, int]]:
if attr_name in keys_to_skip:
continue
atom_dict[attr_name] = attr_val

return atom_dict

@classmethod
Expand Down

0 comments on commit 1395a6b

Please sign in to comment.