From b38799d10c059d8a4c262b4032e2d8c15e1cd846 Mon Sep 17 00:00:00 2001 From: jaclark5 Date: Mon, 10 Feb 2025 13:50:26 -0500 Subject: [PATCH 1/8] Update gitignore to ignore vscode settings files --- .gitignore | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.gitignore b/.gitignore index f1e29222a..8dd4815a8 100644 --- a/.gitignore +++ b/.gitignore @@ -8,6 +8,9 @@ __pycache__/ # OS X extra files *.DS_Store +# VSCode +*.vscode + # Distribution / packaging .Python env/ From b9fb7924fbfaff9b548b7c943e898c4fedbdb946 Mon Sep 17 00:00:00 2001 From: jaclark5 Date: Mon, 10 Feb 2025 13:50:55 -0500 Subject: [PATCH 2/8] Update docstrings in molecule.py relevant to importing and exporting dictionaries --- openff/toolkit/topology/molecule.py | 71 ++++++++++++++++++++--------- 1 file changed, 50 insertions(+), 21 deletions(-) diff --git a/openff/toolkit/topology/molecule.py b/openff/toolkit/topology/molecule.py index fd3918df9..bea60b957 100644 --- a/openff/toolkit/topology/molecule.py +++ b/openff/toolkit/topology/molecule.py @@ -301,7 +301,11 @@ def add_bond(self, bond: "Bond"): self._bonds.append(bond) def to_dict(self) -> dict[str, Union[None, str, int, bool, dict[Any, Any]]]: - """Return a dict representation of the atom.""" + """Return a dict representation of the :class:`Atom` class instance. + + Output dictionary keys and values align with parameters used to initialize + the :class:`Atom` class. + """ # TODO: Should this be implicit in the atom ordering when saved? # atom_dict['molecule_atom_index'] = self._molecule_atom_index return { @@ -316,9 +320,10 @@ def to_dict(self) -> dict[str, Union[None, str, int, bool, dict[Any, Any]]]: @classmethod def from_dict(cls: type[A], atom_dict: dict) -> A: """ - Create an Atom from a dict representation. + Create an :class:`Atom` class instance from a dict representation. - The structure of the dict expected by this function is defined by the output of `Atom.to_dict()`. + The structure of the dict expected by this function is defined by the output of + :meth:`Atom.to_dict()`. """ return cls(**atom_dict) @@ -697,7 +702,10 @@ def __init__( def to_dict(self) -> dict[str, Union[int, bool, str, float]]: """ - Return a dict representation of the bond. + Return a ``dict`` representation of the bond. + + The output dictionary keys and values align with parameters used to initialize + the :class:`Bond` class. """ return { @@ -714,7 +722,8 @@ def from_dict(cls: type[B], molecule: FM, d: dict) -> B: # type: ignore[overrid """ Create a Bond from a dict representation. - The structure of the dict expected by this function is defined by the output of `Bond.to_dict()`. + The structure of the dict expected by this function is defined by the output of + :meth:`Bond.to_dict()`. """ # TODO: This is not used anywhere (`Molecule._initialize_bonds_from_dict()` just calls grabs # the two atoms and calls `Molecule._add_bond`). Remove or change that? @@ -1162,8 +1171,29 @@ def to_dict(self) -> dict: ------- molecule_dict A dictionary representation of the molecule. - - """ + + - **name** (str): An optional name to be associated with the molecule + - **atoms** (list[dict]): A list of dictionary inputs for :meth:`Atom.from_dict()` + - **bonds** (list[dict]): A list of dictionary inputs for :meth:`Bond.from_dict()` + - **conformers** (list[list]): A list containing the cartesian coordinates of each atoms in the order + defined in ``atoms``. + - **properties** (dict): Outputs from chosen a chosen toolkit: + + - **atom_map** (dict): Dictionary of atom index (as in ``atoms`` entry) and the mapped index relevant + to a mapped canonical smiles string + - **\*\*kwargs**: Other toolkit dependent outputs + + - **hierarchy_schemes** (dict[dict]): Dictionary where keys (such as ``"residues"`` and ``"chains"``) + represent dictionary outputs from :meth:`HierarchyScheme.to_dict()` + - **conformers_unit** (float, default="angstrom"): Valid unit of length input for the + `OpenFF Units module `_. + - **partial_charges** (list[float], default=None): Array of partial charge (in elementary charges) + for atoms in the same order as the output,``atoms``. + - **partial_charge_unit** (float, default=None): Valid unit of charge input for the + `OpenFF Units module `_. + If ``partial_charges`` is also included, the default is ``"elementary_charge"`` instead. + + """ from openff.toolkit.utils.utils import serialize_numpy # typing.TypedDict might make this cleaner @@ -1244,17 +1274,16 @@ def from_dict(cls: type[FM], molecule_dict: dict) -> FM: """ Create a new Molecule from a dictionary representation - The structure of the dict expected by this function is defined by the output of `Molecule.to_dict()`. - Parameters ---------- molecule_dict - A dictionary representation of the molecule. + A dictionary representation of the molecule defined by the inputs of + :meth:`Molecule.to_dict()`. Returns ------- molecule - A Molecule created from the dictionary representation + A :class:`Molecule` class instance created from the dictionary representation """ # This implementation is a compromise to let this remain as a classmethod @@ -5876,14 +5905,13 @@ def __init__( Parameters ---------- - parent - The ``Molecule`` to which this scheme belongs. + The :class:`Molecule` to which this scheme belongs. uniqueness_criteria - The names of ``Atom`` metadata entries that define this scheme. An - atom belongs to a ``HierarchyElement`` only if its metadata has the + The names of :class:`Atom` metadata entries that define this scheme. An + atom belongs to a :class:`HierarchyElement` only if its metadata has the same values for these criteria as the other atoms in the - ``HierarchyElement``. + :class:`HierarchyElement`. iterator_name The name of the iterator that will be exposed to access the hierarchy elements generated by this scheme @@ -5917,8 +5945,9 @@ def __init__( self.hierarchy_elements: list[HierarchyElement] = list() def to_dict(self) -> dict: - """ - Serialize this object to a basic dict of strings, ints, and floats + """Serialize this object to a basic dict of strings and lists of ints. + + Keys and values align with parameters used to initialize the :class:`HierarchyScheme` class. """ return_dict: dict[str, Union[str, Sequence[Union[str, int, dict]]]] = dict() return_dict["uniqueness_criteria"] = self.uniqueness_criteria @@ -6064,7 +6093,6 @@ def __init__( Parameters ---------- - scheme The scheme to which this ``HierarchyElement`` belongs identifier @@ -6083,8 +6111,9 @@ def __init__( setattr(self, uniqueness_component, id_component) def to_dict(self) -> dict[str, Union[tuple[Union[str, int]], Sequence[int]]]: - """ - Serialize this object to a basic dict of strings and lists of ints. + """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. """ return { "identifier": self.identifier, From d3e8c80adc110be6689b4c018e7b246eb4fa2700 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 10 Feb 2025 18:55:29 +0000 Subject: [PATCH 3/8] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- openff/toolkit/topology/molecule.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/openff/toolkit/topology/molecule.py b/openff/toolkit/topology/molecule.py index bea60b957..f80d6a1c9 100644 --- a/openff/toolkit/topology/molecule.py +++ b/openff/toolkit/topology/molecule.py @@ -1159,7 +1159,7 @@ def strip_atom_stereochemistry( #################################################################################################### def to_dict(self) -> dict: - """ + r""" Return a dictionary representation of the molecule. .. todo :: @@ -1193,7 +1193,7 @@ def to_dict(self) -> dict: `OpenFF Units module `_. If ``partial_charges`` is also included, the default is ``"elementary_charge"`` instead. - """ + """ from openff.toolkit.utils.utils import serialize_numpy # typing.TypedDict might make this cleaner From dbe8ae0365f9fe967d2046dc80a4181fe61122aa Mon Sep 17 00:00:00 2001 From: jaclark5 Date: Mon, 10 Feb 2025 13:57:07 -0500 Subject: [PATCH 4/8] Update changelog --- docs/releasehistory.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/releasehistory.md b/docs/releasehistory.md index 07fc7bbbd..5a89daf4e 100644 --- a/docs/releasehistory.md +++ b/docs/releasehistory.md @@ -28,6 +28,7 @@ Releases follow the `major.minor.micro` scheme recommended by [PEP440](https://w - [PR #1981](https://github.com/openforcefield/openff-toolkit/pull/1981): Updates documentation to run quicker and use new features, including some in Interchange 0.4. - [PR #2007](https://github.com/openforcefield/openff-toolkit/pull/2007): Documents that the dicts expected by various `from_dict` methods are expected to match the structure of the results of corresponding `to_dict` methods. +- [PR #2018](https://github.com/openforcefield/openff-toolkit/pull/2018): Interlinking of docstrings for further clarity in use of `from_dict` and `to_dict` for `Atom`, `Bond`, and `Molecule` classes. ### Miscellaneous From 8ccd665f36539ba8e5b1dae0e9d5c1279caae3df Mon Sep 17 00:00:00 2001 From: jaclark5 Date: Mon, 10 Feb 2025 14:08:48 -0500 Subject: [PATCH 5/8] formatting --- openff/toolkit/topology/molecule.py | 36 ++++++++++++++--------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/openff/toolkit/topology/molecule.py b/openff/toolkit/topology/molecule.py index f80d6a1c9..462ceb386 100644 --- a/openff/toolkit/topology/molecule.py +++ b/openff/toolkit/topology/molecule.py @@ -302,8 +302,8 @@ def add_bond(self, bond: "Bond"): def to_dict(self) -> dict[str, Union[None, str, int, bool, dict[Any, Any]]]: """Return a dict representation of the :class:`Atom` class instance. - - Output dictionary keys and values align with parameters used to initialize + + Output dictionary keys and values align with parameters used to initialize the :class:`Atom` class. """ # TODO: Should this be implicit in the atom ordering when saved? @@ -319,10 +319,9 @@ def to_dict(self) -> dict[str, Union[None, str, int, bool, dict[Any, Any]]]: @classmethod def from_dict(cls: type[A], atom_dict: dict) -> A: - """ - Create an :class:`Atom` class instance from a dict representation. + """Create an :class:`Atom` class instance from a dict representation. - The structure of the dict expected by this function is defined by the output of + The structure of the dict expected by this function is defined by the output of :meth:`Atom.to_dict()`. """ return cls(**atom_dict) @@ -701,10 +700,9 @@ def __init__( self._stereochemistry = stereochemistry def to_dict(self) -> dict[str, Union[int, bool, str, float]]: - """ - Return a ``dict`` representation of the bond. - - The output dictionary keys and values align with parameters used to initialize + """Return a ``dict`` representation of the bond. + + The output dictionary keys and values align with parameters used to initialize the :class:`Bond` class. """ @@ -722,7 +720,7 @@ def from_dict(cls: type[B], molecule: FM, d: dict) -> B: # type: ignore[overrid """ Create a Bond from a dict representation. - The structure of the dict expected by this function is defined by the output of + The structure of the dict expected by this function is defined by the output of :meth:`Bond.to_dict()`. """ # TODO: This is not used anywhere (`Molecule._initialize_bonds_from_dict()` just calls grabs @@ -1171,28 +1169,28 @@ def to_dict(self) -> dict: ------- molecule_dict A dictionary representation of the molecule. - + - **name** (str): An optional name to be associated with the molecule - **atoms** (list[dict]): A list of dictionary inputs for :meth:`Atom.from_dict()` - **bonds** (list[dict]): A list of dictionary inputs for :meth:`Bond.from_dict()` - **conformers** (list[list]): A list containing the cartesian coordinates of each atoms in the order defined in ``atoms``. - **properties** (dict): Outputs from chosen a chosen toolkit: - + - **atom_map** (dict): Dictionary of atom index (as in ``atoms`` entry) and the mapped index relevant to a mapped canonical smiles string - **\*\*kwargs**: Other toolkit dependent outputs - + - **hierarchy_schemes** (dict[dict]): Dictionary where keys (such as ``"residues"`` and ``"chains"``) represent dictionary outputs from :meth:`HierarchyScheme.to_dict()` - - **conformers_unit** (float, default="angstrom"): Valid unit of length input for the + - **conformers_unit** (float, default="angstrom"): Valid unit of length input for the `OpenFF Units module `_. - **partial_charges** (list[float], default=None): Array of partial charge (in elementary charges) for atoms in the same order as the output,``atoms``. - - **partial_charge_unit** (float, default=None): Valid unit of charge input for the + - **partial_charge_unit** (float, default=None): Valid unit of charge input for the `OpenFF Units module `_. If ``partial_charges`` is also included, the default is ``"elementary_charge"`` instead. - + """ from openff.toolkit.utils.utils import serialize_numpy @@ -1277,7 +1275,7 @@ def from_dict(cls: type[FM], molecule_dict: dict) -> FM: Parameters ---------- molecule_dict - A dictionary representation of the molecule defined by the inputs of + A dictionary representation of the molecule defined by the inputs of :meth:`Molecule.to_dict()`. Returns @@ -5946,7 +5944,7 @@ def __init__( def to_dict(self) -> dict: """Serialize this object to a basic dict of strings and lists of ints. - + Keys and values align with parameters used to initialize the :class:`HierarchyScheme` class. """ return_dict: dict[str, Union[str, Sequence[Union[str, int, dict]]]] = dict() @@ -6112,7 +6110,7 @@ def __init__( def to_dict(self) -> dict[str, Union[tuple[Union[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. """ return { From 2e2c380645ab9d3cbd40db273ea3a4adfc597456 Mon Sep 17 00:00:00 2001 From: Jennifer A Clark Date: Thu, 13 Feb 2025 17:12:35 -0500 Subject: [PATCH 6/8] Fix parameter type in docstring Co-authored-by: Matt Thompson --- openff/toolkit/topology/molecule.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/openff/toolkit/topology/molecule.py b/openff/toolkit/topology/molecule.py index 462ceb386..c88fef17b 100644 --- a/openff/toolkit/topology/molecule.py +++ b/openff/toolkit/topology/molecule.py @@ -1183,11 +1183,11 @@ def to_dict(self) -> dict: - **hierarchy_schemes** (dict[dict]): Dictionary where keys (such as ``"residues"`` and ``"chains"``) represent dictionary outputs from :meth:`HierarchyScheme.to_dict()` - - **conformers_unit** (float, default="angstrom"): Valid unit of length input for the + - **conformers_unit** (str, default="angstrom"): Valid unit of length input for the `OpenFF Units module `_. - **partial_charges** (list[float], default=None): Array of partial charge (in elementary charges) for atoms in the same order as the output,``atoms``. - - **partial_charge_unit** (float, default=None): Valid unit of charge input for the + - **partial_charge_unit** (str, default=None): Valid unit of charge input for the `OpenFF Units module `_. If ``partial_charges`` is also included, the default is ``"elementary_charge"`` instead. From 7f3ae8f50ab4ef2c73f1a7e49e22d2b6f520ef6c Mon Sep 17 00:00:00 2001 From: jaclark5 Date: Thu, 13 Feb 2025 17:25:14 -0500 Subject: [PATCH 7/8] Fix docstring for to_dict --- openff/toolkit/topology/molecule.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/openff/toolkit/topology/molecule.py b/openff/toolkit/topology/molecule.py index c88fef17b..6b04b1147 100644 --- a/openff/toolkit/topology/molecule.py +++ b/openff/toolkit/topology/molecule.py @@ -1173,8 +1173,8 @@ def to_dict(self) -> dict: - **name** (str): An optional name to be associated with the molecule - **atoms** (list[dict]): A list of dictionary inputs for :meth:`Atom.from_dict()` - **bonds** (list[dict]): A list of dictionary inputs for :meth:`Bond.from_dict()` - - **conformers** (list[list]): A list containing the cartesian coordinates of each atoms in the order - defined in ``atoms``. + - **conformers** (list[list[float]]): A list containing the cartesian coordinates of each atom in units + of ``conformer_unit`` in the order defined in ``atoms``. - **properties** (dict): Outputs from chosen a chosen toolkit: - **atom_map** (dict): Dictionary of atom index (as in ``atoms`` entry) and the mapped index relevant @@ -1185,8 +1185,8 @@ def to_dict(self) -> dict: represent dictionary outputs from :meth:`HierarchyScheme.to_dict()` - **conformers_unit** (str, default="angstrom"): Valid unit of length input for the `OpenFF Units module `_. - - **partial_charges** (list[float], default=None): Array of partial charge (in elementary charges) - for atoms in the same order as the output,``atoms``. + - **partial_charges** (list[float], default=None): Array of partial charge (in unit defined by + ``partial_charge_unit``) for atoms in the same order as the output,``atoms``. - **partial_charge_unit** (str, default=None): Valid unit of charge input for the `OpenFF Units module `_. If ``partial_charges`` is also included, the default is ``"elementary_charge"`` instead. From 0f3b6a113d753afe0baab658bf12e29c8e5cc8d4 Mon Sep 17 00:00:00 2001 From: jaclark5 Date: Thu, 13 Feb 2025 17:34:04 -0500 Subject: [PATCH 8/8] Update docstring --- openff/toolkit/topology/molecule.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openff/toolkit/topology/molecule.py b/openff/toolkit/topology/molecule.py index 6b04b1147..97b6f9198 100644 --- a/openff/toolkit/topology/molecule.py +++ b/openff/toolkit/topology/molecule.py @@ -1175,7 +1175,7 @@ def to_dict(self) -> dict: - **bonds** (list[dict]): A list of dictionary inputs for :meth:`Bond.from_dict()` - **conformers** (list[list[float]]): A list containing the cartesian coordinates of each atom in units of ``conformer_unit`` in the order defined in ``atoms``. - - **properties** (dict): Outputs from chosen a chosen toolkit: + - **properties** (dict): Outputs from a chosen toolkit: - **atom_map** (dict): Dictionary of atom index (as in ``atoms`` entry) and the mapped index relevant to a mapped canonical smiles string