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

Annotate most everything #1798

Merged
merged 22 commits into from
Mar 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
1080538
Annotate everything
mattwthompson Dec 19, 2023
4d75f2e
Shift more annotations
mattwthompson Dec 19, 2023
7262655
Fix more typing errors
mattwthompson Dec 20, 2023
03d06e0
Remove more (most?) annotations from docstrings
mattwthompson Dec 20, 2023
355b1f7
Merge remote-tracking branch 'upstream/main' into shift-annotations
mattwthompson Dec 20, 2023
8bf9ea0
Remove debug code
mattwthompson Dec 20, 2023
4c42433
Merge remote-tracking branch 'upstream/main' into shift-annotations
mattwthompson Feb 22, 2024
df605d9
Merge remote-tracking branch 'upstream/main' into shift-annotations
mattwthompson Feb 28, 2024
84ee68c
Update
mattwthompson Feb 28, 2024
1d7a875
Merge remote-tracking branch 'upstream/main' into shift-annotations
mattwthompson Mar 8, 2024
0d5ed3a
Merge remote-tracking branch 'upstream/main' into shift-annotations
mattwthompson Mar 21, 2024
631102a
Apply suggestions from code review
mattwthompson Mar 21, 2024
ed07ace
Updates from code review
mattwthompson Mar 21, 2024
87a1105
Merge remote-tracking branch 'upstream/shift-annotations' into shift-…
mattwthompson Mar 21, 2024
e00fc41
Updates/fix
mattwthompson Mar 21, 2024
e920598
Fix
mattwthompson Mar 21, 2024
913d93f
Update openff/toolkit/topology/topology.py
mattwthompson Mar 21, 2024
7f8a3b1
Merge remote-tracking branch 'upstream/shift-annotations' into shift-…
mattwthompson Mar 21, 2024
d2256f4
Fix
mattwthompson Mar 21, 2024
874cca1
Fix
mattwthompson Mar 21, 2024
e1203fe
Avoid name clash
mattwthompson Mar 21, 2024
45c7876
Update release history
mattwthompson Mar 21, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .github/workflows/CI.yml
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,8 @@ jobs:
run: pytest -r fE --tb=short openff/toolkit/_tests/test_links.py

- name: Run mypy
if: ${{ matrix.rdkit == true && matrix.openeye == true }}
# subtle differences in Python/mypy versions, just keep it passing on one
if: ${{ matrix.rdkit == true && matrix.openeye && matrix.python-version == 3.11 }}
run: mypy -p "openff.toolkit"

- name: Run unit tests
Expand Down
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ ci:
files: ^openff|(^examples/((?!deprecated).)*$)|^docs
repos:
- repo: https://github.com/psf/black
rev: 24.1.1
rev: 24.2.0
hooks:
- id: black
- id: black-jupyter
Expand Down
2 changes: 1 addition & 1 deletion devtools/conda-envs/test_env.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ dependencies:
- nbval
# No idea why this is necessary, see https://github.com/openforcefield/openff-toolkit/pull/1821
- nomkl
- mypy
- mypy =1.8
- typing_extensions
- pydantic =1
- pip:
Expand Down
2 changes: 2 additions & 0 deletions docs/releasehistory.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ Releases follow the `major.minor.micro` scheme recommended by [PEP440](https://w

## Current development

* #1798 Adds type annotations to most of the codebase.

### API-breaking changes

### Behavior changes
Expand Down
52 changes: 33 additions & 19 deletions openff/toolkit/topology/_mm_molecule.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

"""

from typing import TYPE_CHECKING, Generator, NoReturn, Optional, Union
from typing import TYPE_CHECKING, Generator, Iterable, NoReturn, Optional, Union

from openff.units.elements import MASSES, SYMBOLS

Expand All @@ -37,7 +37,7 @@ class _SimpleMolecule:
def __init__(self):
self.atoms = []
self.bonds = []
self.conformers = None
self._conformers = None
self._hierarchy_schemes = dict()

def add_atom(self, atomic_number: int, **kwargs):
Expand All @@ -61,10 +61,14 @@ def add_bond(self, atom1, atom2, **kwargs):
bond = _SimpleBond(atom1_atom, atom2_atom, **kwargs)
self.bonds.append(bond)

@property
def conformers(self) -> Optional[list["Quantity"]]:
return self._conformers

def add_conformer(self, conformer):
if self.conformers is None:
self.conformers = list()
self.conformers.append(conformer)
if self._conformers is None:
self._conformers = list()
self._conformers.append(conformer)

@property
def n_atoms(self) -> int:
Expand All @@ -76,9 +80,7 @@ def n_bonds(self) -> int:

@property
def n_conformers(self) -> int:
if self.conformers is None:
return 0
return len(self.conformers)
return 0 if self._conformers is None else len(self._conformers)

def atom(self, index):
return self.atoms[index]
Expand Down Expand Up @@ -305,6 +307,15 @@ def _from_subgraph(cls, subgraph: "nx.Graph"):

return molecule

# TODO: Implement me - shouldn't need to be too different than Molecule.add_hierarchy_scheme
# since the extra chemical information is unrelated to hierarchy/metadata
def add_hierarchy_scheme(
self,
uniqueness_criteria: Iterable[str],
iterator_name: str,
) -> NoReturn:
mattwthompson marked this conversation as resolved.
Show resolved Hide resolved
raise NotImplementedError()

def to_dict(self) -> dict:
molecule_dict = dict()
special_serialization_logic = [
Expand All @@ -325,14 +336,14 @@ def to_dict(self) -> dict:

molecule_dict["bonds"] = [bond.to_dict() for bond in self.bonds]

if self.conformers is None:
if self._conformers is None:
molecule_dict["conformers"] = None
else:
molecule_dict["conformers"] = []
molecule_dict["conformers_unit"] = (
"angstrom" # Have this defined as a class variable?
)
for conf in self.conformers:
for conf in self._conformers:
conf_unitless = conf.m_as(unit.angstrom)
conf_serialized, conf_shape = serialize_numpy((conf_unitless))
molecule_dict["conformers"].append(conf_serialized)
Expand Down Expand Up @@ -363,15 +374,15 @@ def from_dict(cls, molecule_dict):

conformers = molecule_dict.pop("conformers")
if conformers is None:
molecule.conformers = None
molecule._conformers = None
else:
conformers_unit = molecule_dict.pop("conformers_unit")
molecule.conformers = list()
molecule._conformers = list()
for ser_conf in conformers:
conformers_shape = (molecule.n_atoms, 3)
conformer_unitless = deserialize_numpy(ser_conf, conformers_shape)
conformer = unit.Quantity(conformer_unitless, conformers_unit)
molecule.conformers.append(conformer)
molecule._conformers.append(conformer)

hier_scheme_dicts = molecule_dict.pop("hierarchy_schemes")
for iterator_name, hierarchy_scheme_dict in hier_scheme_dicts.items():
Expand All @@ -387,7 +398,8 @@ def from_dict(cls, molecule_dict):
atom_indices=element_dict["atom_indices"],
)

{setattr(molecule, key, val) for key, val in molecule_dict.items()}
for key, val in molecule_dict.items():
setattr(molecule, key, val)

return molecule

Expand All @@ -407,11 +419,13 @@ def from_molecule(cls, molecule: Molecule):
atom2=mm_molecule.atom(bond.atom2_index),
)

mm_molecule.conformers = molecule.conformers
if molecule.conformers is not None:
for conformer in molecule.conformers:
mm_molecule.add_conformer(conformer)

for name, hierarchy_scheme in molecule.hierarchy_schemes.items():
assert name == hierarchy_scheme.iterator_name
mm_molecule.add_hierarchy_scheme( # type: ignore[operator]
mm_molecule.add_hierarchy_scheme(
uniqueness_criteria=hierarchy_scheme.uniqueness_criteria,
iterator_name=hierarchy_scheme.iterator_name,
)
Expand Down Expand Up @@ -503,7 +517,7 @@ def generate_unique_atom_names(self):
"""Generate unique atom names. See `Molecule.generate_unique_atom_names`."""
from collections import defaultdict

element_counts = defaultdict(int)
element_counts: defaultdict[str, int] = defaultdict(int)
for atom in self.atoms:
symbol = SYMBOLS[atom.atomic_number]
element_counts[symbol] += 1
Expand All @@ -528,7 +542,7 @@ def __deepcopy__(self, memo):
return self.__class__.from_dict(self.to_dict())


_SimpleMolecule.add_hierarchy_scheme = Molecule.add_hierarchy_scheme # type: ignore[attr-defined]
_SimpleMolecule.add_hierarchy_scheme = Molecule.add_hierarchy_scheme # type: ignore[assignment]
_SimpleMolecule.update_hierarchy_schemes = Molecule.update_hierarchy_schemes # type: ignore[attr-defined]


Expand All @@ -550,7 +564,7 @@ def __init__(
self._name = name
self._atomic_number = atomic_number
self._molecule = molecule
self._bonds: list[Optional[_SimpleBond]] = list()
self._bonds: list[_SimpleBond] = list()
for key, val in kwargs.items():
setattr(self, key, val)

Expand Down
Loading