Update `Molecule.from_qcschema`, notebook #1763

mattwthompson · 2023-11-08T16:19:37Z

review-notebook-app · 2023-11-08T16:19:42Z

Check out this pull request on

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

devtools/conda-envs/openeye-examples.yaml

mattwthompson · 2023-11-15T19:10:21Z

openff/toolkit/_tests/test_molecule.py

+
+        client = qcportal.PortalClient("https://api.qcarchive.molssi.org:443")
+
+        record = [*client.query_molecules(molecular_formula="C16H20N3O5")][-1]


This was previously [0], which regularly grabbed a record that did not have CMILES. There's probably a different index that's safer, since this assumes that all future records will have CMILES that the underlying toolkits can assign stereochemistry to, but that assumption seems relatively safe.

codecov · 2023-11-15T19:12:49Z

Codecov Report

Merging #1763 (d0bb1f5) into main (9b23790) will increase coverage by 0.03%.
Report is 8 commits behind head on main.
The diff coverage is 98.07%.

Additional details and impacted files

mattwthompson · 2023-11-15T21:59:18Z

openff/toolkit/topology/molecule.py

-                        "The provided client can not query molecules, make sure it is an instance of"
-                        "qcportal.client.FractalClient() with the correct address."
-                    )
+                        "The provided client failed to query molecules, make sure it is an instance of"
+                        "`qcportal.PortalClient` and pointed to the correct address."
+                    ) from error


I tried a bit to cover this in a test, but had difficulties getting a bad client that would fail in this particular way.

j-wags · 2023-12-02T02:09:19Z

examples/QCArchive_interface/QCarchive_interface.ipynb

@@ -6,7 +6,7 @@
   "source": [


Entries store much information about the molecule, but not the geometry.
This doesn't appear to be true. Did this change in 0.50? entry.initial_molecule is a 3D QC mol with coordinates and everything. Maybe 0.50 will require changes to Molecule.from_qcschema, or maybe there was a reason we didn't take coords from entries? I need to come back to this when I continue this review.

Reply via ReviewNB

No clue, I'm going off of this docstring which implies an initial geometry cannot be fetched when client=None

openff-toolkit/openff/toolkit/topology/molecule.py

Lines 4603 to 4605 in 9b23790

client : optional, default=None,

A qcportal.FractalClient instance to use for fetching an initial geometry.

Only used if ``qca_record`` is a dataset entry.

Coming back to this... yup, this changed in QCFractal. It's a good change, but I'll need to update the from_qcschema method.

Old QCF (naively fetched entries DON'T have geometry):

New QCF (naively fetched entries DO have geometry):

for more information, see https://pre-commit.ci

docs/releasehistory.md

Co-authored-by: Jeff Wagner <jeffrey.wagner+github@openforcefield.org>

mattwthompson

I looked through the source code (did not deeply pass over the QCSchema notebook or unit tests) and have non-blocking comments/suggestions

openff/toolkit/topology/molecule.py

j-wags

Excellent. Thanks for the second set of eyes - You caught a lot of stuff that could be improved!

openff/toolkit/topology/molecule.py

Co-authored-by: Matt Thompson <mattwthompson@protonmail.com>

j-wags · 2024-01-18T21:44:55Z

And so they're not lost to history - I took a few notes on this refactor here

Update Molecule.from_qcshema, notebook

1e02018

mattwthompson added 10 commits November 8, 2023 14:26

Update some tests

16b580d

Update more tests

a31c615

Drop annotation

fa29737

Update tests

2496f60

Merge remote-tracking branch 'upstream/main' into new-qcarchive

9a39d2d

Update

aaea8d9

Separate QCArchive tests

97ee4f6

Work around QCPortal/OpenEye import bug

5a23bd6

Try using most recent record

ab2ca20

Install NGLview

5aa49b7

mattwthompson commented Nov 15, 2023

View reviewed changes

devtools/conda-envs/openeye-examples.yaml Show resolved Hide resolved

mattwthompson commented Nov 15, 2023

View reviewed changes

mattwthompson added 6 commits November 15, 2023 13:13

Re-run notebook

16ff0ab

Fix doctests

2b4c7e1

Update cookbook

eb88022

Expand linting to doc notebooks

2800c38

Fiddle with notebook setup

abfe360

Typo

19ad9dc

mattwthompson commented Nov 15, 2023

View reviewed changes

mattwthompson marked this pull request as ready for review November 16, 2023 17:37

mattwthompson requested review from Yoshanuikabundi and j-wags November 16, 2023 17:38

j-wags reviewed Dec 2, 2023

View reviewed changes

j-wags and others added 3 commits December 1, 2023 18:11

cleanups and changes to qcarchive interface notebook

24a8a4d

[pre-commit.ci] auto fixes from pre-commit.com hooks

514d3d0

for more information, see https://pre-commit.ci

Merge remote-tracking branch 'upstream/main' into new-qcarchive

0b2cf14

j-wags changed the title ~~Update Molecule.from_qcshema, notebook~~ Update Molecule.from_qcschema, notebook Dec 11, 2023

j-wags and others added 11 commits January 8, 2024 18:54

initial update of from_qcschema

013a900

update docstrings, add/update tests, lint

b58cc04

update qcarchive interface notebook

518c08f

remove offline molecule test, remove unused code

3a445fc

Merge branch 'main' into new-qcarchive

61be960

[pre-commit.ci] auto fixes from pre-commit.com hooks

2881eb3

for more information, see https://pre-commit.ci

fix tests and make mypy happy

1d40923

fix tests and make mypy happy

8d97762

try to make linters happy

06b3a3e

don't lint docs notebooks

65d93b3

Update releasehistory for api change

b733a54

j-wags reviewed Jan 17, 2024

View reviewed changes

docs/releasehistory.md Outdated Show resolved Hide resolved

mattwthompson and others added 3 commits January 17, 2024 14:36

Merge remote-tracking branch 'upstream/new-qcarchive' into new-qcarchive

c6aa4c6

Update docs/releasehistory.md

57754cf

Co-authored-by: Jeff Wagner <jeffrey.wagner+github@openforcefield.org>

Lint notebooks in automation

e8f3418

mattwthompson commented Jan 17, 2024

View reviewed changes

j-wags reviewed Jan 18, 2024

View reviewed changes

openff/toolkit/topology/molecule.py Outdated Show resolved Hide resolved

j-wags and others added 2 commits January 18, 2024 10:48

Apply suggestions from code review

5fd8e75

Co-authored-by: Matt Thompson <mattwthompson@protonmail.com>

fix docs for flake errors

d0bb1f5

j-wags merged commit b479325 into main Jan 24, 2024
18 checks passed

j-wags deleted the new-qcarchive branch January 24, 2024 00:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update `Molecule.from_qcschema`, notebook #1763

Update `Molecule.from_qcschema`, notebook #1763

mattwthompson commented Nov 8, 2023 •

edited by j-wags

Loading

review-notebook-app bot commented Nov 8, 2023

mattwthompson Nov 15, 2023

codecov bot commented Nov 15, 2023 •

edited

Loading

mattwthompson Nov 15, 2023

j-wags Dec 2, 2023 •

edited

Loading

mattwthompson Dec 2, 2023

j-wags Dec 4, 2023

mattwthompson left a comment

j-wags left a comment

j-wags commented Jan 18, 2024


		client = qcportal.PortalClient("https://api.qcarchive.molssi.org:443")

		record = [*client.query_molecules(molecular_formula="C16H20N3O5")][-1]

	client : optional, default=None,
	A qcportal.FractalClient instance to use for fetching an initial geometry.
	Only used if ``qca_record`` is a dataset entry.

Update Molecule.from_qcschema, notebook #1763

Update Molecule.from_qcschema, notebook #1763

Conversation

mattwthompson commented Nov 8, 2023 • edited by j-wags Loading

review-notebook-app bot commented Nov 8, 2023

mattwthompson Nov 15, 2023

Choose a reason for hiding this comment

codecov bot commented Nov 15, 2023 • edited Loading

Codecov Report

mattwthompson Nov 15, 2023

Choose a reason for hiding this comment

j-wags Dec 2, 2023 • edited Loading

Choose a reason for hiding this comment

mattwthompson Dec 2, 2023

Choose a reason for hiding this comment

j-wags Dec 4, 2023

Choose a reason for hiding this comment

mattwthompson left a comment

Choose a reason for hiding this comment

j-wags left a comment

Choose a reason for hiding this comment

j-wags commented Jan 18, 2024

Update `Molecule.from_qcschema`, notebook #1763

Update `Molecule.from_qcschema`, notebook #1763

mattwthompson commented Nov 8, 2023 •

edited by j-wags

Loading

codecov bot commented Nov 15, 2023 •

edited

Loading

j-wags Dec 2, 2023 •

edited

Loading