conda-forge / rdkit-feedstock

A conda-smithy repository for rdkit.
BSD 3-Clause "New" or "Revised" License
8 stars 22 forks source link

update to new release #80

Closed greglandrum closed 2 years ago

greglandrum commented 2 years ago

Checklist

conda-forge-linter commented 2 years ago

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.

greglandrum commented 2 years ago

@conda-forge-admin, please rerender

mcs07 commented 2 years ago

Same timeouts that were seen in #79? I'm not sure why these are taking >6 hours now - looking at the last release they were all 3-4 hours?

jaimergp commented 2 years ago

We might need to cross-compile for PPC, but it'd be better to fix the vendored dependencies problem first.

mcs07 commented 2 years ago

I agree that unvendoring would be good, but might take quite a bit of effort. Some should be mostly straightforward, like InChI, coordgen and maeparser, but I believe that others like RapidJSON and AvalonTools will actually require changes to the RDKit CMake configuration to allow for an externally provided library.

It would be really nice to get the 2019.09.1 release out as soon as possible... how does everyone feel about merging this PR now and then working on fixing the PPC packages afterwards?

mcs07 commented 2 years ago

I was just going to see if I could quickly start off with an external InChI library, but then I noticed that the conda-forge package is missing Windows builds. It looks like I made a PR a couple of years ago to add them but it never got merged: https://github.com/conda-forge/inchi-feedstock/pull/2

mcs07 commented 2 years ago

Here's the CMake configuration that I implemented in Open Babel that allows for an external RapidJSON library:

https://github.com/openbabel/openbabel/blob/master/CMakeLists.txt#L604-L631 https://github.com/openbabel/openbabel/blob/master/cmake/modules/FindRapidJSON.cmake

I think that RDKit would need something similar, which would probably be best contributed upstream rather than as a patch in this feedstock.

There is already a config like this for maeparser and coordgen, so the main task there would be to submit new recipes for them via staged-recipes.

jaimergp commented 2 years ago

Agreed that it's ok to release the new RDKit like this, provided that unvendoring happens in a new build later. PPC can be disabled until then.

mcs07 commented 2 years ago

@conda-forge-admin, please rerender

greglandrum commented 2 years ago

Agreed that it's ok to release the new RDKit like this, provided that unvendoring happens in a new build later. PPC can be disabled until then.

Thanks! for what it's worth, the usage of the linux_ppc stuff does seem to be pretty low. Check slides 4 and 5 here: https://github.com/rdkit/UGM_2021/blob/main/Presentations/Landrum_StateOfTheToolkit.pdf

jaimergp commented 2 years ago

I guess we can enable freesasa here while we are at it :)

greglandrum commented 2 years ago

I just noticed that I messed up the release numbering in the main RDKit CMakeLists.txt file. I'm going to do a 2021.09.2 release with that fixed after I've had a chance to verify that I actually have it right now.

mcs07 commented 2 years ago

We could maybe catch this in future by adding something like this to the conda recipe:

tests:
  commands:
    - python -c "import sys; import rdkit; sys.exit(0) if rdkit.__version__ == '{{ version }}' else sys.exit(1)"
jaimergp commented 2 years ago

Or more concise:

tests:
  commands:
    - python -c "import rdkit; assert rdkit.__version__ == '{{ version }}'"