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

python 3.12 builds reference python 3.10 in the RDKit cmake configuration #170

Open
1 task done
greglandrum opened this issue Aug 16, 2024 · 5 comments · May be fixed by #171
Open
1 task done

python 3.12 builds reference python 3.10 in the RDKit cmake configuration #170

greglandrum opened this issue Aug 16, 2024 · 5 comments · May be fixed by #171
Labels

Comments

@greglandrum
Copy link
Contributor

Solution to issue cannot be found in the documentation.

  • I checked the documentation.

Issue

Here's a demonstration of the problem:

(rdkit_distbug) glandrum@stoat:/tmp$ grep INTERFACE_INCLUDE_DIRECTORIES /home/glandrum/mambaforge/envs/rdkit_distbug/lib/cmake/rdkit/rdkit-targets.cmake 
  INTERFACE_INCLUDE_DIRECTORIES "/home/glandrum/mambaforge/envs/rdkit_distbug/include/python3.10;/home/glandrum/mambaforge/envs/rdkit_distbug/lib/python3.10/site-packages/numpy/_core/include;${_IMPORT_PREFIX}/include/rdkit;/home/glandrum/mambaforge/envs/rdkit_distbug/include"
  INTERFACE_INCLUDE_DIRECTORIES "/home/glandrum/mambaforge/envs/rdkit_distbug/include/freetype2"
(rdkit_distbug) glandrum@stoat:/tmp$ conda list -n rdkit_distbug rdkit
# packages in environment at /home/glandrum/mambaforge/envs/rdkit_distbug:
#
# Name                    Version                   Build  Channel
librdkit                  2024.03.5            h79cfef2_3    conda-forge
rdkit                     2024.03.5       py312h7b4b7d0_3    conda-forge

Here's the first build of v2024.03.5:

(rdkit_distbug) glandrum@stoat:/tmp$ grep INTERFACE_INCLUDE_DIRECTORIES /home/glandrum/mambaforge/envs/py312_rdkit/lib/cmake/rdkit/rdkit-targets.cmake 
  INTERFACE_INCLUDE_DIRECTORIES "/home/glandrum/mambaforge/envs/py312_rdkit/include/python3.12;/home/glandrum/mambaforge/envs/py312_rdkit/lib/python3.12/site-packages/numpy/_core/include;${_IMPORT_PREFIX}/include/rdkit;/home/glandrum/mambaforge/envs/py312_rdkit/include"
  INTERFACE_INCLUDE_DIRECTORIES "/home/glandrum/mambaforge/envs/py312_rdkit/include/freetype2"
(rdkit_distbug) glandrum@stoat:/tmp$ conda list -n py312_rdkit rdkit
# packages in environment at /home/glandrum/mambaforge/envs/py312_rdkit:
#
# Name                    Version                   Build  Channel
rdkit                     2024.03.5       py312hf6fb7c8_0    conda-forge

That one looks ok

I will try and track down which build this started with in order to help figure out what happened.

Installed packages

see above

Environment info

see above
@greglandrum
Copy link
Contributor Author

Ok, that was easy. The first problematic build on linux is py312h7b4b7d0_1
That was done on July 24, so it was PR #158 :

@skearnes
Copy link
Contributor

skearnes commented Aug 16, 2024 via email

@greglandrum
Copy link
Contributor Author

Ah... I think I have this figured out: rdkit-targets.cmake is part of the librdkit deliverable, and that doesn't have a python version associated with it. Currently there are things in that package that do have a python version dependency though

It doesn't feel right to have the python dependency in librdkit, so I'm going to see if this is something we can fix upstream

greglandrum added a commit to greglandrum/rdkit-feedstock that referenced this issue Aug 16, 2024
@greglandrum greglandrum linked a pull request Aug 16, 2024 that will close this issue
4 tasks
@skearnes
Copy link
Contributor

Should this approach for fixing this be removing anything python-related from librdkit as opposed to bringing in python-specific things? The base build itself should respect the python version; maybe we have to fix that instead?

@rvianello
Copy link

I was looking a bit into this issue yesterday, and I think the python dependency in librdkit is motivated by the RDBoost library code around Boost Python and numpy which is then linked/used by other python extension modules.

Due to some implementation details in the RDKit cmake build, this python dependency is actually broader than needed and extends to all C++ libraries. I'm exploring some ways to cleanup things a bit, but as long as RDBoost is distributed as part of librdkit, multiple package variants targeting the supported versions of python might be actually needed.

(I'm still not sure about the available solutions, but I was thinking that RDBoost could be perhaps packaged together with the rdkit python extensions and code, and if it's not expected to be used to build 3rd-party extensions, maybe the RDBoost cmake target is not required to be exported, or it could be exported using a dedicated cmake config file..).

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

Successfully merging a pull request may close this issue.

3 participants