Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

0.10.0 build 1 and examples package #9

Merged
merged 23 commits into from
Aug 2, 2021

Conversation

Yoshanuikabundi
Copy link
Contributor

@Yoshanuikabundi Yoshanuikabundi commented Apr 22, 2021

The openff-toolkit repository has a number of example notebooks that demonstrate its use, and we're about to put a bunch of effort into improving them and adding more. In openforcefield/openff-toolkit#888, I investigated adding tooling to simplify the use of the examples, and we discovered that they were not distributed with the toolkit by Conda. This PR adds a new subpackage, openff-toolkit-examples, that includes the examples themselves as well as depending on the packages required to run them. In the future, this package may also include a helper application to run the examples, but using this packaging method allows us to let Conda manage package management.

Checklist

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

I do have some suggestions for making it better though...

For recipe:

  • It looks like the 'openff-toolkit-examples' output doesn't have any tests.

@Yoshanuikabundi Yoshanuikabundi changed the title Add examples guess [DNM] Examples package Apr 22, 2021
@Yoshanuikabundi
Copy link
Contributor Author

@j-wags I got it working! With this PR, there is a openff-toolkit-examples package that installs the dependencies of the examples, and installs the examples themselves to $CONDA_PREFIX/share/openff-toolkit/examples.

My questions:

  • Is that the best path to hold the examples?
  • What tests should I add? Is just replicating all the notebook tests from upstream appropriate? I think the point is just to make sure all the dependencies are there and all the files are present but not sure what the best way to accomplish this is, since the aforementioned tests take about 10 minutes.

@j-wags
Copy link
Contributor

j-wags commented Apr 29, 2021

To keep things in one place -- Discussion and next steps are in today's meeting notes

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I was trying to look for recipes to lint for you, but it appears we have a merge conflict.
Please try to merge or rebase with the base branch to resolve this conflict.

Please ping the 'conda-forge/core' team (using the @ notation in a comment) if you believe this is a bug.

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@Yoshanuikabundi
Copy link
Contributor Author

*Wipes sweat from brow

Added the test checking that the installed file tree matches the source tree. It seems to be working!

Copy link
Contributor

@j-wags j-wags left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should let us test from a branch of the toolkit -- We need to make sure to point this back to the release URL before we merge!!

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I was trying to look for recipes to lint for you, but it appears we have a merge conflict.
Please try to merge or rebase with the base branch to resolve this conflict.

Please ping the 'conda-forge/core' team (using the @ notation in a comment) if you believe this is a bug.

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@Yoshanuikabundi
Copy link
Contributor Author

This is all updated and working with the latest version of the examples helper as of this comment. PR for the helper: openforcefield/openff-toolkit#934

I did notice that the packages all still depend on python >= 3.6. Should we update this to python >= 3.7 while we're here?

Copy link
Contributor

@j-wags j-wags left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great. Thanks for all the iterating on this @Yoshanuikabundi!

I think we need to merge this BEFORE the next release. As long as we revert the version+build, there shouldn't be any packages built or tests run. I'll give it a shot now :-)

@j-wags j-wags changed the title [DNM] Examples package Examples package May 13, 2021
@j-wags
Copy link
Contributor

j-wags commented May 13, 2021

@conda-forge-admin, please rerender

@j-wags
Copy link
Contributor

j-wags commented Aug 2, 2021

@conda-forge-admin, please rerender

conda-forge-linter and others added 3 commits August 2, 2021 18:01
@j-wags j-wags changed the title Examples package 0.10.0 build 1 and examples package Aug 2, 2021
@j-wags j-wags merged commit 0239a2b into conda-forge:master Aug 2, 2021
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants