diff --git a/docs/releasehistory.md b/docs/releasehistory.md index 99c16fde6..d685802c8 100644 --- a/docs/releasehistory.md +++ b/docs/releasehistory.md @@ -14,6 +14,8 @@ Releases follow the `major.minor.micro` scheme recommended by [PEP440](https://w ### Bugfixes - [PR #2052](https://github.com/openforcefield/openff-toolkit/pull/2052): Fixes bug where `Topology.from_pdb` couldn't load NH4+ ([Issue #2051](https://github.com/openforcefield/openff-toolkit/issues/2051)) +- [PR #2000](https://github.com/openforcefield/openff-toolkit/pull/2000): Assorted type annotation fixes +- [PR #2000](https://github.com/openforcefield/openff-toolkit/pull/2000): `Molecule.to_rdkit()` no longer raises an exception when converting a molecule with non-decimal residue numbers to RDKit; instead, the RDKit residue number is simply silently left unset. ### Miscellaneous diff --git a/openff/toolkit/_tests/test_toolkits.py b/openff/toolkit/_tests/test_toolkits.py index bf3683b22..8b9a36e68 100644 --- a/openff/toolkit/_tests/test_toolkits.py +++ b/openff/toolkit/_tests/test_toolkits.py @@ -3508,6 +3508,26 @@ def test_to_rdkit_losing_aromaticity_(self): for offatom, rdatom in zip(mol.atoms, rdmol.GetAtoms()): assert offatom.is_aromatic is rdatom.GetIsAromatic() + def test_to_rdkit_str_resnum(self): + smiles = ("O") + + mol = Molecule.from_smiles(smiles) + + # Test an int, a string that can convert to an int, and a non-intable str + atom_resnums = [9998, "9999", "A000"] + # RDKit's default residue number is 0 + expected_atom_resnums = [9998, 9999, 0] + + for atom, resnum in zip(mol.atoms, atom_resnums): + atom.metadata["residue_number"] = resnum + atom.metadata["residue_name"] = "HOH" + + rdmol = mol.to_rdkit() + + # now make sure the residue number matches for each atom + for resnum, rdatom in zip(expected_atom_resnums, rdmol.GetAtoms()): + assert rdatom.GetPDBResidueInfo().GetResidueNumber() == resnum + @pytest.mark.slow def test_max_substructure_matches_can_handle_large_molecule(self): """Test RDKitToolkitWrapper substructure search handles more than the default of maxMatches = 1000 diff --git a/openff/toolkit/topology/_mm_molecule.py b/openff/toolkit/topology/_mm_molecule.py index 42cd31f65..6181af3ca 100644 --- a/openff/toolkit/topology/_mm_molecule.py +++ b/openff/toolkit/topology/_mm_molecule.py @@ -82,13 +82,13 @@ def n_bonds(self) -> int: def n_conformers(self) -> int: return 0 if self._conformers is None else len(self._conformers) - def atom(self, index): + def atom(self, index: int) -> "_SimpleAtom": return self.atoms[index] def atom_index(self, atom) -> int: return self.atoms.index(atom) - def bond(self, index): + def bond(self, index: int) -> "_SimpleBond": return self.bonds[index] def get_bond_between(self, atom1_index, atom2_index): @@ -369,7 +369,8 @@ def from_dict(cls, molecule_dict): atom1_index = bond_dict["atom1_index"] atom2_index = bond_dict["atom2_index"] molecule.add_bond( - atom1=molecule.atom(atom1_index), atom2=molecule.atom(atom2_index) + atom1=molecule.atom(atom1_index), + atom2=molecule.atom(atom2_index) ) conformers = molecule_dict.pop("conformers") diff --git a/openff/toolkit/topology/molecule.py b/openff/toolkit/topology/molecule.py index 572efce01..f3ff35a77 100644 --- a/openff/toolkit/topology/molecule.py +++ b/openff/toolkit/topology/molecule.py @@ -31,7 +31,7 @@ import pathlib import warnings from collections import UserDict, defaultdict -from collections.abc import Generator, Iterable, Sequence +from collections.abc import Generator, Iterable, Iterator, Mapping, MutableMapping, Sequence from copy import deepcopy from functools import cmp_to_key from typing import ( @@ -92,6 +92,7 @@ import IPython.display import networkx as nx import nglview + from openmm.unit import Quantity as OMMQuantity from rdkit.Chem import Mol as RDMol from openff.toolkit.topology._mm_molecule import _SimpleAtom, _SimpleMolecule @@ -159,7 +160,7 @@ def molecule_particle_index(self) -> int: """ Returns the index of this particle in its molecule """ - return self._molecule.atoms.index(self) + return self._molecule.atoms.index(self) # type:ignore @property def name(self) -> str: @@ -226,8 +227,8 @@ def __init__( is_aromatic: bool, name: Optional[str] = None, molecule=None, - stereochemistry: Optional[str] = None, - metadata: Optional[dict[str, Union[int, str]]] = None, + stereochemistry: Literal["R", "S", None] = None, + metadata: Mapping[str, int | str] | None = None, ): """ Create an immutable Atom object. @@ -272,7 +273,7 @@ def __init__( # Use the setter here, since it will handle either ints or Quantities # and it is designed to quickly process ints - self.formal_charge = formal_charge + self.formal_charge = formal_charge # type: ignore[assignment] self._is_aromatic = is_aromatic self._stereochemistry = stereochemistry if name is None: @@ -312,9 +313,13 @@ def to_dict(self) -> dict[str, Union[None, str, int, bool, dict[Any, Any]]]: """ # TODO: Should this be implicit in the atom ordering when saved? # atom_dict['molecule_atom_index'] = self._molecule_atom_index + + # trust that the unit is e + formal_charge: int = self._formal_charge.m # type: ignore + return { "atomic_number": self._atomic_number, - "formal_charge": self._formal_charge.m, # Trust that the unit is e + "formal_charge": formal_charge, "is_aromatic": self._is_aromatic, "stereochemistry": self._stereochemistry, "name": self._name, @@ -331,21 +336,21 @@ def from_dict(cls: type[A], atom_dict: dict) -> A: return cls(**atom_dict) @property - def metadata(self): + def metadata(self) -> MutableMapping[str, int | str]: """ The atom's metadata dictionary """ return self._metadata @property - def formal_charge(self): + def formal_charge(self) -> Quantity: """ The atom's formal charge """ return self._formal_charge @formal_charge.setter - def formal_charge(self, other): + def formal_charge(self, other: "int | Quantity | OMMQuantity"): """ Set the atom's formal charge. Accepts either ints or unit-wrapped ints with units of charge. """ @@ -432,19 +437,20 @@ def is_aromatic(self): return self._is_aromatic @property - def stereochemistry(self): + def stereochemistry(self) -> Literal["R", "S", None]: """ The atom's stereochemistry (if defined, otherwise None) """ return self._stereochemistry @stereochemistry.setter - def stereochemistry(self, value: Literal["CW", "CCW", None]): + def stereochemistry(self, value: Literal["R", "S", None]): """Set the atoms stereochemistry Parameters ---------- value - The stereochemistry around this atom, allowed values are "CW", "CCW", or None, + The stereochemistry around this atom, suggested values are ``"R"``, + ``"S"``, or ``None``. """ # if (value != 'CW') and (value != 'CCW') and not(value is None): @@ -762,11 +768,11 @@ def atoms(self): return (self._atom1, self._atom2) @property - def bond_order(self): + def bond_order(self) -> int: return self._bond_order @bond_order.setter - def bond_order(self, value): + def bond_order(self, value: int): if isinstance(value, int): self._bond_order = value else: @@ -787,7 +793,7 @@ def fractional_bond_order(self, value): self._fractional_bond_order = value @property - def stereochemistry(self): + def stereochemistry(self) -> Literal["E", "Z", None]: return self._stereochemistry @property @@ -1208,6 +1214,7 @@ def to_dict(self) -> dict: list[str], list[bytes], list[HierarchyElement], + list[dict[str, Any]], ], ] = dict() molecule_dict["name"] = self._name @@ -1385,9 +1392,9 @@ def _initialize(self): """ Clear the contents of the current molecule. """ - self._name = "" - self._atoms = list() - self._bonds = list() # list of bonds between Atom objects + self._name: str = "" + self._atoms: list[Atom] = list() + self._bonds: list[Bond] = list() # list of bonds between Atom objects self._properties = {} # Attached properties to be preserved # self._cached_properties = None # Cached properties (such as partial charges) can be recomputed as needed self._partial_charges = None @@ -2002,8 +2009,8 @@ def _is_exactly_the_same_as(self, other): @staticmethod def are_isomorphic( - mol1: Union["FrozenMolecule", "_SimpleMolecule", "nx.Graph"], - mol2: Union["FrozenMolecule", "_SimpleMolecule", "nx.Graph"], + mol1: "FrozenMolecule | _SimpleMolecule | nx.Graph[int]", + mol2: "FrozenMolecule | _SimpleMolecule | nx.Graph[int]", return_atom_map: bool = False, aromatic_matching: bool = True, formal_charge_matching: bool = True, @@ -2012,7 +2019,7 @@ def are_isomorphic( bond_stereochemistry_matching: bool = True, strip_pyrimidal_n_atom_stereo: bool = True, toolkit_registry: TKR = GLOBAL_TOOLKIT_REGISTRY, - ) -> tuple[bool, Optional[dict[int, int]]]: + ) -> tuple[bool, None | dict[int, int]]: """ Determine if ``mol1`` is isomorphic to ``mol2``. @@ -2230,8 +2237,15 @@ def to_networkx(data: Union[FrozenMolecule, nx.Graph]) -> nx.Graph: def is_isomorphic_with( self, - other: Union["FrozenMolecule", "_SimpleMolecule", "nx.Graph"], - **kwargs, + other: "FrozenMolecule | _SimpleMolecule | nx.Graph[int]", + aromatic_matching: bool = True, + formal_charge_matching: bool = True, + bond_order_matching: bool = True, + atom_stereochemistry_matching: bool = True, + bond_stereochemistry_matching: bool = True, + strip_pyrimidal_n_atom_stereo: bool = True, + toolkit_registry: TKR = GLOBAL_TOOLKIT_REGISTRY, + **kwargs: dict[str, Any], ) -> bool: """ Check if the molecule is isomorphic with the other molecule which can be an openff.toolkit.topology.Molecule @@ -2244,13 +2258,13 @@ def is_isomorphic_with( other aromatic_matching - compare the aromatic attributes of bonds and atoms. + compare the aromatic attributes of bonds and atoms. formal_charge_matching - compare the formal charges attributes of the atoms. + compare the formal charges attributes of the atoms. bond_order_matching - compare the bond order on attributes of the bonds. + compare the bond order on attributes of the bonds. atom_stereochemistry_matching If ``False``, atoms' stereochemistry is ignored for the @@ -2269,6 +2283,9 @@ def is_isomorphic_with( :class:`ToolkitRegistry` or :class:`ToolkitWrapper` to use for removing stereochemistry from pyrimidal nitrogens. + **kwargs + Ignored. This will be removed in an upcoming release. + Returns ------- isomorphic @@ -2278,29 +2295,23 @@ def is_isomorphic_with( self, other, return_atom_map=False, - aromatic_matching=kwargs.get("aromatic_matching", True), - formal_charge_matching=kwargs.get("formal_charge_matching", True), - bond_order_matching=kwargs.get("bond_order_matching", True), - atom_stereochemistry_matching=kwargs.get( - "atom_stereochemistry_matching", True - ), - bond_stereochemistry_matching=kwargs.get( - "bond_stereochemistry_matching", True - ), - strip_pyrimidal_n_atom_stereo=kwargs.get( - "strip_pyrimidal_n_atom_stereo", True - ), - toolkit_registry=kwargs.get("toolkit_registry", GLOBAL_TOOLKIT_REGISTRY), + aromatic_matching=aromatic_matching, + formal_charge_matching=formal_charge_matching, + bond_order_matching=bond_order_matching, + atom_stereochemistry_matching=atom_stereochemistry_matching, + bond_stereochemistry_matching=bond_stereochemistry_matching, + strip_pyrimidal_n_atom_stereo=strip_pyrimidal_n_atom_stereo, + toolkit_registry=toolkit_registry, )[0] def generate_conformers( self, toolkit_registry: TKR = GLOBAL_TOOLKIT_REGISTRY, n_conformers: int = 10, - rms_cutoff: Optional[Quantity] = None, + rms_cutoff: Quantity | None = None, clear_existing: bool = True, make_carboxylic_acids_cis: bool = True, - ): + ) -> None: """ Generate conformers for this molecule using an underlying toolkit. @@ -2474,9 +2485,7 @@ def dihedral(a): dihedrals.shape = (n_conformers, n_cooh_groups, 1, 1) # Get indices of trans COOH groups - trans_indices = np.logical_not( - np.logical_and((-np.pi / 2) < dihedrals, dihedrals < (np.pi / 2)) - ) + trans_indices = np.logical_not(np.logical_and((-np.pi / 2) < dihedrals, dihedrals < (np.pi / 2))) # Expand array so it can be used to index cooh_xyz trans_indices = np.repeat(trans_indices, repeats=4, axis=2) trans_indices = np.repeat(trans_indices, repeats=3, axis=3) @@ -2517,9 +2526,7 @@ def apply_elf_conformer_selection( self, percentage: float = 2.0, limit: int = 10, - toolkit_registry: Optional[ - Union[ToolkitRegistry, ToolkitWrapper] - ] = GLOBAL_TOOLKIT_REGISTRY, + toolkit_registry: TKR = GLOBAL_TOOLKIT_REGISTRY, **kwargs, ): """Select a set of diverse conformers from the molecule's conformers with ELF. @@ -2879,7 +2886,7 @@ def to_networkx(self) -> "nx.Graph": """ import networkx as nx - G: nx.classes.graph.Graph = nx.Graph() + G: nx.classes.graph.Graph[int] = nx.Graph() for atom in self.atoms: G.add_node( atom.molecule_atom_index, @@ -2984,9 +2991,9 @@ def _add_atom( atomic_number: int, formal_charge: int, is_aromatic: bool, - stereochemistry: Optional[str] = None, - name: Optional[str] = None, - metadata=None, + stereochemistry: Literal["R", "S", None] = None, + name: str | None = None, + metadata: dict[str, int | str] | None = None, invalidate_cache: bool = True, ) -> int: """ @@ -3054,12 +3061,12 @@ def _add_atom( def _add_bond( self, - atom1, - atom2, - bond_order, - is_aromatic, - stereochemistry=None, - fractional_bond_order=None, + atom1: int | Atom, + atom2: int | Atom, + bond_order: int, + is_aromatic: bool, + stereochemistry: Literal["E", "Z", None] = None, + fractional_bond_order: float | None = None, invalidate_cache: bool = True, ): """ @@ -3155,7 +3162,7 @@ def _add_conformer(self, coordinates: Quantity): if not isinstance(coordinates, openmm_unit.Quantity): raise IncompatibleUnitError( "Unsupported type passed to Molecule._add_conformer setter. " - "Found object of type {type(other)}." + f"Found object of type {type(coordinates)}." ) if not coordinates.unit.is_compatible(openmm_unit.meter): @@ -3299,7 +3306,7 @@ def n_impropers(self) -> int: return len(self._impropers) @property - def atoms(self): + def atoms(self) -> list[Atom]: """ Iterate over all Atom objects in the molecule. """ @@ -3604,7 +3611,7 @@ def to_hill_formula(self) -> str: return self._hill_formula @staticmethod - def _object_to_hill_formula(obj: Union["FrozenMolecule", "nx.Graph"]) -> str: + def _object_to_hill_formula(obj: Union["FrozenMolecule", "nx.Graph[int]"]) -> str: """Take a Molecule or NetworkX graph and generate its Hill formula. This provides a backdoor to the old functionality of Molecule.to_hill_formula, which was a static method that duck-typed inputs of Molecule or graph objects.""" @@ -3836,11 +3843,11 @@ def to_topology(self): @classmethod def from_file( cls: type[FM], - file_path: Union[str, pathlib.Path, TextIO], - file_format=None, - toolkit_registry=GLOBAL_TOOLKIT_REGISTRY, + file_path: str | pathlib.Path | TextIO, + file_format: str | None = None, + toolkit_registry: TKR = GLOBAL_TOOLKIT_REGISTRY, allow_undefined_stereo: bool = False, - ) -> Union[FM, list[FM]]: + ) -> FM | list[FM]: """ Create one or more molecules from a file @@ -4240,8 +4247,8 @@ def to_file(self, file_path, file_format, toolkit_registry=GLOBAL_TOOLKIT_REGIST supported_formats = {} for _toolkit in toolkit_registry.registered_toolkits: supported_formats[_toolkit.toolkit_name] = ( - _toolkit.toolkit_file_write_formats - ) + _toolkit.toolkit_file_write_formats + ) raise ValueError( f"The requested file format ({file_format}) is not available from any of the installed toolkits " f"(supported formats: {supported_formats})" @@ -4565,8 +4572,8 @@ def to_qcschema(self, multiplicity=1, conformer=0, extras=None): symbols = [SYMBOLS[atom.atomic_number] for atom in self.atoms] if extras is not None: extras["canonical_isomeric_explicit_hydrogen_mapped_smiles"] = ( - self.to_smiles(mapped=True) - ) + self.to_smiles(mapped=True) + ) else: extras = { "canonical_isomeric_explicit_hydrogen_mapped_smiles": self.to_smiles( @@ -5072,7 +5079,7 @@ def remap( for i in range(self.n_atoms): # get the old atom info old_atom = self._atoms[new_to_cur[i]] - new_molecule._add_atom(**old_atom.to_dict()) + new_molecule._add_atom(**old_atom.to_dict()) # type:ignore # this is the first time we access the mapping; catch an index error # here corresponding to mapping that starts from 0 or higher except (KeyError, IndexError): @@ -5086,7 +5093,7 @@ def remap( bond_dict = bond.to_dict() bond_dict["atom1"] = atoms[0] bond_dict["atom2"] = atoms[1] - new_molecule._add_bond(**bond_dict) + new_molecule._add_bond(**bond_dict) # type:ignore # we can now resort the bonds sorted_bonds = sorted( @@ -5288,8 +5295,7 @@ def get_bond_between(self, i: Union[int, Atom], j: Union[int, Atom]) -> Bond: else: raise TypeError( "Invalid input passed to get_bond_between(). Expected ints or Atoms, " - f"got {j} and {j}." - ) + f"got {j} and {j}.") for bond in atom_i.bonds: for atom in bond.atoms: @@ -5371,7 +5377,7 @@ def add_atom( atomic_number: int, formal_charge: int, is_aromatic: bool, - stereochemistry: Optional[str] = None, + stereochemistry: Literal["R", "S", None] = None, name: Optional[str] = None, metadata: Optional[dict[str, Union[int, str]]] = None, ) -> int: @@ -5437,7 +5443,7 @@ def add_bond( atom2: Union[int, "Atom"], bond_order: int, is_aromatic: bool, - stereochemistry: Optional[str] = None, + stereochemistry: Literal["E", "Z", None] = None, fractional_bond_order: Optional[float] = None, ) -> int: """ @@ -5719,7 +5725,7 @@ def perceive_residues( smarts_no_chirality ] = substructure_dictionary_no_chirality[res_name].pop( smarts - ) # update key + ) # update key # replace with the new substructure dictionary substructure_dictionary = substructure_dictionary_no_chirality @@ -5768,14 +5774,10 @@ def perceive_residues( # Now the matches have been deduplicated and de-subsetted for residue_num, match_dict in enumerate(all_matches): for smarts_idx, atom_idx in enumerate(match_dict["atom_idxs"]): - self.atoms[atom_idx].metadata["residue_name"] = match_dict[ - "residue_name" - ] + self.atoms[atom_idx].metadata["residue_name"] = match_dict["residue_name"] self.atoms[atom_idx].metadata["residue_number"] = str(residue_num + 1) self.atoms[atom_idx].metadata["insertion_code"] = " " - self.atoms[atom_idx].metadata["atom_name"] = match_dict["atom_names"][ - smarts_idx - ] + self.atoms[atom_idx].metadata["atom_name"] = match_dict["atom_names"][smarts_idx] # Now add the residue hierarchy scheme self._add_residue_hierarchy_scheme() @@ -5799,7 +5801,7 @@ def _ipython_display_(self): # pragma: no cover pass -def _networkx_graph_to_hill_formula(graph: "nx.Graph") -> str: +def _networkx_graph_to_hill_formula(graph: "nx.Graph[int]") -> str: """ Convert a NetworkX graph to a Hill formula. @@ -5820,7 +5822,7 @@ def _networkx_graph_to_hill_formula(graph: "nx.Graph") -> str: raise ValueError("The graph must be a NetworkX graph.") atom_nums = list(dict(graph.nodes(data="atomic_number", default=1)).values()) - return _atom_nums_to_hill_formula(atom_nums) + return _atom_nums_to_hill_formula(atom_nums) # type:ignore[arg-type] def _atom_nums_to_hill_formula(atom_nums: list[int]) -> str: @@ -5856,9 +5858,7 @@ def _atom_nums_to_hill_formula(atom_nums: list[int]) -> str: def _nth_degree_neighbors_from_graphlike( graphlike: MoleculeLike, n_degrees: int, -) -> Generator[ - Union[tuple[Atom, Atom], tuple["_SimpleAtom", "_SimpleAtom"]], None, None -]: +) -> Iterator[tuple[Atom, Atom] | tuple["_SimpleAtom", "_SimpleAtom"]]: """ Given a graph-like object, return a tuple of the nth degree neighbors of each atom. @@ -6007,11 +6007,9 @@ def perceive_hierarchy(self): self.hierarchy_elements = list() # Determine which atoms should get added to which HierarchyElements - hier_eles_to_add: defaultdict[tuple[Union[int, str]], list[Atom]] = ( - defaultdict(list) - ) + hier_eles_to_add: defaultdict[tuple[int | str, ...], list[Atom]] = defaultdict(list) for atom in self.parent.atoms: - _atom_key = list() + _atom_key: list[int | str] = list() for field_key in self.uniqueness_criteria: if field_key in atom.metadata: _atom_key.append(atom.metadata[field_key]) @@ -6029,7 +6027,7 @@ def perceive_hierarchy(self): def add_hierarchy_element( self, - identifier: tuple[Union[str, int]], + identifier: tuple[str | int, ...], atom_indices: Sequence[int], ) -> "HierarchyElement": """ @@ -6116,7 +6114,7 @@ class HierarchyElement: def __init__( self, scheme: HierarchyScheme, - identifier: tuple[Union[str, int]], + identifier: tuple[str | int, ...], atom_indices: Sequence[int], ): """ @@ -6141,7 +6139,7 @@ def __init__( ): setattr(self, uniqueness_component, id_component) - def to_dict(self) -> dict[str, Union[tuple[Union[str, int]], Sequence[int]]]: + def to_dict(self) -> dict[str, tuple[str | int, ...] | Sequence[int]]: """Serialize this object to a basic dict of strings and lists of ints. Keys and values align with parameters used to initialize the :class:`HierarchyElement` class. @@ -6159,7 +6157,7 @@ def n_atoms(self) -> int: return len(self.atom_indices) @property - def atoms(self) -> Generator["Atom", None, None]: + def atoms(self) -> Iterator["Atom"]: """ Iterator over the atoms in this hierarchy element. """ @@ -6213,7 +6211,7 @@ def generate_unique_atom_names(self, suffix: str = "x"): def _has_unique_atom_names( - obj: Union[FrozenMolecule, "_SimpleMolecule", HierarchyElement] + obj: "FrozenMolecule | _SimpleMolecule | HierarchyElement", ) -> bool: """``True`` if the object has unique atom names, ``False`` otherwise.""" unique_atom_names = set([atom.name for atom in obj.atoms]) @@ -6223,8 +6221,9 @@ def _has_unique_atom_names( def _generate_unique_atom_names( - obj: Union[FrozenMolecule, HierarchyElement], suffix: str = "x" -): + obj: FrozenMolecule | HierarchyElement, + suffix: str = "x", +) -> None: """ Generate unique atom names from the element symbol and count. diff --git a/openff/toolkit/topology/topology.py b/openff/toolkit/topology/topology.py index be98c001d..1ed0a27b0 100644 --- a/openff/toolkit/topology/topology.py +++ b/openff/toolkit/topology/topology.py @@ -525,7 +525,7 @@ def n_unique_molecules(self) -> int: @classmethod def from_molecules( cls, - molecules: Union[MoleculeLike, list[MoleculeLike]], + molecules: MoleculeLike | Iterable[MoleculeLike], ) -> "Topology": """ Create a new Topology object containing one copy of each of the specified molecule(s). @@ -703,7 +703,7 @@ def n_molecules(self) -> int: return len(self._molecules) @property - def molecules(self) -> Generator[MoleculeLike, None, None]: + def molecules(self) -> Iterator[MoleculeLike]: """Returns an iterator over all the Molecules in this Topology Returns @@ -1085,7 +1085,7 @@ def chemical_environment_matches( topology_atom_indices = [] for molecule_atom_index in match: atom = mol_instance.atom(atom_map[molecule_atom_index]) - topology_atom_indices.append(self.atom_index(atom)) + topology_atom_indices.append(self.atom_index(atom)) # type: ignore[arg-type] environment_match = Topology._ChemicalEnvironmentMatch( tuple(match), unique_mol, tuple(topology_atom_indices) @@ -1800,7 +1800,7 @@ def from_pdb( [[*vec3.value_in_unit(openmm_unit.angstrom)] for vec3 in pdb.getPositions()] ) - topology = toolkit_registry.call( + topology: Topology = toolkit_registry.call( "_polymer_openmm_pdbfile_to_offtop", cls, pdb, @@ -1809,7 +1809,7 @@ def from_pdb( _custom_substructures, ) - for off_atom, atom in zip([*topology.atoms], pdb.topology.atoms()): + for off_atom, atom in zip(topology.atoms, pdb.topology.atoms()): off_atom.metadata["residue_name"] = atom.residue.name off_atom.metadata["residue_number"] = atom.residue.id off_atom.metadata["insertion_code"] = atom.residue.insertionCode @@ -2135,7 +2135,7 @@ def clear_positions(self): for molecule in self.molecules: molecule._conformers = None - def set_positions(self, array: Quantity): + def set_positions(self, array: Quantity) -> None: """ Set the positions in a topology by copying from a single (n, 3) array. @@ -2166,9 +2166,9 @@ def set_positions(self, array: Quantity): # Copy the array in nanometers and make it an OpenFF Quantity array = Quantity(np.asarray(array.to(unit.nanometer).magnitude), unit.nanometer) - if array.shape != (self.n_atoms, 3): # type: ignore[attr-defined] + if array.shape != (self.n_atoms, 3): raise WrongShapeError( - f"Array has shape {array.shape} but should have shape {self.n_atoms, 3}" # type: ignore[attr-defined] + f"Array has shape {array.shape} but should have shape {self.n_atoms, 3}" ) start = 0 @@ -2343,7 +2343,8 @@ def atom(self, atom_topology_index: int) -> "Atom": if next_molecule_start_index > atom_topology_index: atom_molecule_index = atom_topology_index - this_molecule_start_index # NOTE: the index here should still be in the topology index order, NOT the reference molecule's - return molecule.atom(atom_molecule_index) + # can Molecule.atom be a _SimpleAtom? + return molecule.atom(atom_molecule_index) # type: ignore[return-value] this_molecule_start_index += molecule.n_atoms raise AtomNotInTopologyError( diff --git a/openff/toolkit/utils/builtin_wrapper.py b/openff/toolkit/utils/builtin_wrapper.py index 5cc6daa34..2666e163a 100644 --- a/openff/toolkit/utils/builtin_wrapper.py +++ b/openff/toolkit/utils/builtin_wrapper.py @@ -131,7 +131,7 @@ def assign_partial_charges( partial_charges = [0.0] * molecule.n_atoms elif partial_charge_method == "formal_charge": - partial_charges = [float(atom.formal_charge.m) for atom in molecule.atoms] + partial_charges = [float(atom.formal_charge.m) for atom in molecule.atoms] # type: ignore molecule.partial_charges = Quantity(partial_charges, unit.elementary_charge) diff --git a/openff/toolkit/utils/rdkit_wrapper.py b/openff/toolkit/utils/rdkit_wrapper.py index deb7ca8fe..4918d7c3b 100644 --- a/openff/toolkit/utils/rdkit_wrapper.py +++ b/openff/toolkit/utils/rdkit_wrapper.py @@ -2716,8 +2716,14 @@ def to_rdkit( res.SetResidueName(atom.metadata["residue_name"]) if "residue_number" in atom.metadata: - atom_has_any_metadata = True - res.SetResidueNumber(int(atom.metadata["residue_number"])) + try: + residue_number_int = int(atom.metadata["residue_number"]) + except ValueError: + # Residue number is a string that could not be converted to int + pass + else: + atom_has_any_metadata = True + res.SetResidueNumber(residue_number_int) if "insertion_code" in atom.metadata: atom_has_any_metadata = True @@ -3267,7 +3273,7 @@ def _detect_undefined_stereo( atom1, atom2 = bond.GetBeginAtom(), bond.GetEndAtom() msg += ( f" - Bond {undefined_bond_idx} (atoms {atom1.GetIdx()}-{atom2.GetIdx()} of element " - "({atom1.GetSymbol()}-{atom2.GetSymbol()})\n" + f"({atom1.GetSymbol()}-{atom2.GetSymbol()})\n" ) raise UndefinedStereochemistryError(err_msg_prefix + msg)