Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Add openmmforcefields and perses #13448

Merged
merged 11 commits into from
Feb 10, 2021
Merged

Conversation

jaimergp
Copy link
Member

@jaimergp jaimergp commented Dec 11, 2020

Checklist

  • Title of this PR is meaningful: e.g. "Adding my_nifty_package", not "updated meta.yaml"
  • License file is packaged (see here for an example)
  • Source is from official source
  • Package does not vendor other packages. (If a package uses the source of another package, they should be separate packages or the licenses of all packages need to be packaged)
  • If static libraries are linked in, the license of the static library is packaged.
  • Build number is 0
  • A tarball (url) rather than a repo (e.g. git_url) is used in your recipe (see here for more details)
  • GitHub users listed in the maintainer section have posted a comment confirming they are willing to be listed there
  • When in trouble, please check our knowledge base documentation before pinging a team.

Supersedes #13234

@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 (recipes/clusterutils, recipes/openff-evaluator, recipes/openff-forcefields, recipes/openff-recharge, recipes/openff-smirnoff99frosst, recipes/openff-toolkit, recipes/openmmforcefields, recipes/openmmtools, recipes/openmoltools, recipes/perses, recipes/yank) and found it was in an excellent condition.

Copy link
Contributor

@SimonBoothroyd SimonBoothroyd left a comment

Choose a reason for hiding this comment

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

LGTM after the changes.

recipes/openff-recharge/meta.yaml Outdated Show resolved Hide resolved
recipes/openff-recharge/meta.yaml Outdated Show resolved Hide resolved
recipes/openff-recharge/meta.yaml Outdated Show resolved Hide resolved
recipes/openff-recharge/meta.yaml Outdated Show resolved Hide resolved
recipes/openff-recharge/meta.yaml Outdated Show resolved Hide resolved
Co-authored-by: SimonBoothroyd <simon.boothroyd@colorado.edu>
@mattwthompson
Copy link
Member

I agree to be a maintainer of the recipes listed by my name.

Copy link
Member

@mattwthompson mattwthompson left a comment

Choose a reason for hiding this comment

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

Just a few changes as a result of the 0.8.1 release earlier in the week

Thanks for handling everything!

recipes/openff-toolkit/meta.yaml Outdated Show resolved Hide resolved
recipes/openff-toolkit/meta.yaml Outdated Show resolved Hide resolved
host:
- python
- pip
run:
Copy link
Member

Choose a reason for hiding this comment

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

Also needs mdtraj (long story, will be fixed in next release)

@j-wags
Copy link

j-wags commented Dec 11, 2020

I agree to be a maintainer of openff-evaluator, openff-forcefields, openff-recharge, openff-smirnoff99frosst, openff-toolkit.

I'd like to keep from publishing these packages on conda-forges main label until we return from the holidays in January. I'd be happy to publish on a non-main label for testing sooner.

@peastman
Copy link
Contributor

I agree to be a maintainer.

@jchodera
Copy link
Contributor

I agree to be a maintainer!

Co-authored-by: Matt Thompson <mattwthomp@gmail.com>
Co-authored-by: SimonBoothroyd <simon.boothroyd@colorado.edu>
@mattwthompson
Copy link
Member

mattwthompson commented Dec 14, 2020

Given that we have some packages being renamed, and users might be installing these into existing environments with differently-named-but-the-same-software packages from omnia, should we add some run_constrained lines? (Thanks to @SimonBoothroyd for suggesting this!)

@jaimergp
Copy link
Member Author

Should we add some run_constrained lines?

Oh, yes, this should be added!

@mattwthompson
Copy link
Member

mattwthompson commented Jan 5, 2021

Based on some discussion in #13234, some serialization deps for the toolkit can be trimmed down now that is openforcefield/openff-toolkit#794 merged (still pending a release)

@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 (recipes/openff-evaluator, recipes/openff-forcefields, recipes/openff-recharge, recipes/openff-smirnoff99frosst, recipes/openff-toolkit, recipes/openmmforcefields, recipes/perses) and found it was in an excellent condition.

@jaimergp
Copy link
Member Author

jaimergp commented Jan 9, 2021

Note this will fail until #13635 is merged

@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 (recipes/openff-evaluator, recipes/openff-recharge, recipes/openmmforcefields, recipes/perses) and found it was in an excellent condition.

@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 (recipes/openmmforcefields, recipes/perses) and found it was in an excellent condition.

@jaimergp jaimergp changed the title Migrate openmm-depending packages from omnia Add openmmforcefields and perses Jan 26, 2021
@jaimergp
Copy link
Member Author

jaimergp commented Feb 8, 2021

@jchodera Can you check if the dependencies for openmmforcefields (and their constraints) are accurate?

@mikemhenry
Copy link
Contributor

It looks like the windows build is failing to find ambertools but ambertools doesn't have any windows builds. If we don't have any intention to support windows for openmmforcefields then we can drop the windows builds.

noarch: python
number: 0
script: "{{ PYTHON }} -m pip install . -vv --no-deps"

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
skip: True # [win]

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this will work to skip windows builds/testing

Copy link
Member Author

Choose a reason for hiding this comment

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

We can't use platform selectors for skip if noarch: python is set. It is expected that Windows will fail for a noarch if dependencies are missing!

@jchodera
Copy link
Contributor

jchodera commented Feb 8, 2021

@jchodera Can you check if the dependencies for openmmforcefields (and their constraints) are accurate?

@jaimergp: The dependencies appear to be accurate!

If there's no ambertools windows build, we can skip win builds for now.

@jaimergp
Copy link
Member Author

jaimergp commented Feb 9, 2021

@conda-forge/help-python-c, this is ready for review, thanks!

- openmmtools >=0.20.0
- yank
- pdbfixer
# - openeye-toolkits
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason why this isn't being added to conda-forge? (point me to a comment if there already is one)

Copy link
Member Author

@jaimergp jaimergp Feb 10, 2021

Choose a reason for hiding this comment

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

Licensing issues, sadly. They are only available through the company's channel.

We work around that requirement using try-guarded imports for that package, and make sure to mention the extra package in the errors plus documentation.

@chrisburr chrisburr merged commit 984fc11 into conda-forge:master Feb 10, 2021
@jaimergp
Copy link
Member Author

Thanks @chrisburr!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants