search

openforcefield / openff-qcsubmit

Automated tools for submitting molecules to QCFractal
https://openff-qcsubmit.readthedocs.io/en/latest/index.html
MIT License
26 stars 4 forks source link

Enable examples CI #283

Closed j-wags closed 3 weeks ago

j-wags commented 1 month ago

Todos

Status

ntBre commented 1 month ago

This commit should get creating-datasets.ipynb working, but retrieving-results.ipynb is failing at a call to BasicResultCollection.visualize, which was removed in #141 without comment as far as I can tell. Should this API be restored? Alternatively, its code could be in-lined into the notebook, or that cell of the notebook could just be deleted.

j-wags commented 4 weeks ago

I'd go for in-lining it in this case. It looks like the heavy lifting was done in molecules_to_pdf from openff.qcsubmit.utils.visualize which is still in the codebase, so that should work great.

j-wags commented 3 weeks ago

Oh, and the socket thing is kinda weird, but it drives an important point home so I don't really mind it. We can improve it in the future if we feel inspired, but this PR is already enough of an improvement to merge.

ntBre commented 3 weeks ago

Awesome, thanks for the review! I updated the release history and added a small preview_dataset function to print only the first 5 entries in each dataset. I thought this might look nicer than slicing the output, but it does copy the dataset so I'm happy to go for the fix you proposed if you prefer.

j-wags commented 3 weeks ago

Looks great. It's a little complex but I don't know of an easier way to make it cleaner, and it's way better than it was before. Thanks @ntBre!

j-wags commented 3 weeks ago

This is good to merge whenever you're ready :-)