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

WIP: Packaging with Meson #363

Conversation

ye11owSub
Copy link

@ye11owSub ye11owSub commented May 19, 2024

I moved the build process to use the pyproject.toml file

Why it needs to be done:

  • It will be possible to add dependencies for the build step (such as numpy)
  • There will be a more modern way to manage linters, runtime dependencies and meta-information about the project.

Why has the current project form become a problem:

  • pyproject.toml with steuptools as a backend doesn't have a standard approach to building C extensions in parallel.
  • Parsing all possible paths for libraries can be resource-intensive and difficult to maintain.

I've chosen Meson because:

  • Meson is a fast and user-friendly build system.
  • No need for parsing files - Meson searches for dependencies itself and downloads them when needed.
  • Pymol is a project that relies heavily on C/CPP extensions. So, I checked other projects in similar situations. Projects like scipy, numpy, scikit-learn and many more use Meson as their build system.

@ye11owSub ye11owSub force-pushed the adding_packaging_with_pyproject branch 2 times, most recently from 3a5884c to ee901ab Compare May 19, 2024 23:24
@JarrettSJohnson
Copy link
Member

JarrettSJohnson commented May 19, 2024

Thanks for the PR; I was also in the middle of working on this last weekend 👍. Other stuff came up so I didn't get to finish it.

Understanding that this is still WIP, just a couple of requests:

  1. Keep create_shadertext.py unremoved. It's good to have for debugging purposes and other repos depend on this.
  2. Also keep the monkeypatch_distutils.py unremoved for now
  3. Revise the github action workflow file to use the modern installation approach.

I explained in a previous PR, but the reason we haven't made the full transition yet was to allow for parallel compilation, which the monkeypatch_distutils script provided. This would be restored via https://github.com/schrodinger/pymol-open-source/tree/cmake . I'll be soon merging this. (EDIT: this is now merged)

@ye11owSub ye11owSub force-pushed the adding_packaging_with_pyproject branch 4 times, most recently from 2b922a2 to c9b7877 Compare May 20, 2024 15:53
@ye11owSub ye11owSub force-pushed the adding_packaging_with_pyproject branch 6 times, most recently from fccc77f to d3889f6 Compare May 30, 2024 21:02
@ye11owSub ye11owSub force-pushed the adding_packaging_with_pyproject branch 14 times, most recently from e6447d1 to 468f565 Compare June 5, 2024 22:29
@JarrettSJohnson
Copy link
Member

Hi @ye11owSub , I haven't been following too closely, but I was curious about what added benefit of having meson build here replacing CMake. The changes seem pretty widespread across all of the python module directories, and changing all of these would also need to applied to Incentive PyMOL as well to maintain parity, which would be a lot more work for us as well considering we also have other dependencies that are not visible here. With regards to scope of the change for migrating away from distutils, is this required for a pyproject.toml?

@ye11owSub ye11owSub force-pushed the adding_packaging_with_pyproject branch from 468f565 to 6b1ea7c Compare June 5, 2024 22:45
@ye11owSub ye11owSub force-pushed the adding_packaging_with_pyproject branch 3 times, most recently from c678ceb to 939ba74 Compare June 5, 2024 22:55
@ye11owSub
Copy link
Author

ye11owSub commented Jun 6, 2024

Hi @JarrettSJohnson!

No, this is not necessary when switching to project.toml. However, it seems like a reasonable step to update the project in this direction.
Also i can't agree that the project itself has to be changed a lot. All you need to do is add a mason.build file and list the source files.

I've tried to explain my perspective in more detail in the description

@ye11owSub ye11owSub force-pushed the adding_packaging_with_pyproject branch 3 times, most recently from fb12a49 to 0797c6e Compare June 6, 2024 11:28
@ye11owSub ye11owSub force-pushed the adding_packaging_with_pyproject branch from 0797c6e to 0a7ffb0 Compare June 6, 2024 11:31
@JarrettSJohnson
Copy link
Member

JarrettSJohnson commented Jun 6, 2024

Thanks for updating your description. I've spoken to others about this; and we really do appreciate a lot of work that you've put in to the PR draft so far. Though, I just wanted to state this before getting too deep with progressing with Meson. Because of the large surface area of which this affects not only this codebase but other proprietary, internal ones, we would need to first take some time to investigate and develop for it first before changes like this can be propagated to this public repository.

I think it's perhaps prudent to split this work into at least two PRs:

  1. If possible, pyproject.toml support for the current C++ extensions build approach with CMake (which are currently in master); the work there, I believe, is small enough to where that can be merged relatively quickly.
  2. Update build system to use Meson. I think by most accounts that it is superior to what's currently done, but for reasons stated above, we cannot yet merge this in anytime soon. Because of developer schedules, probably will be at least another couple to a few months.

@ye11owSub
Copy link
Author

Hi @JarrettSJohnson

I tried to build a project using setuptools as the backend, but I encountered several issues:

  1. The create_shadertext.py script could not be imported. You can read more about the reasons for this here.
    A possible solution would be to move the contents of create_shadertext.py to setup.py or update sys.path.

  2. I have not found a way to pass custom arguments to the build process. To my knowledge, there is no standard approach for this (meson provides a build_options.txt file for that purpose)
    A possible solution would be to utilize environment variables.

If you would like to try it out for yourself, please feel free to use this PR.

What do you think about it?

@JarrettSJohnson
Copy link
Member

Thanks! I haven't forgotten about this, but will briefly be unavailable until July. I'll review this afterward. Thanks!

@bionicles
Copy link

bionicles commented Jul 6, 2024

Would it be possible to get the pyproject.toml working and post wheels on pypi?

a "pip install pymol" and "pip install -e ." for the repo, would enable more broad use of the project because it could be installed like a normal python project without too much special OS-specific treatment or needing to dive into docs and copy paste a bunch of commands

and the editable install can enable developers to tinker and see results immediately and contribute those results upstream when they are good

if anyone needs it, here is an install script which works on ubuntu 22.04 on wsl2 with python 3.12 today, and should be fairly easy to get working on mac

#!/bin/bash

# this script is designed to for DEBIAN/UBUNTU and MAC
# windows is too hard, sorry

# https://pymolwiki.org/index.php/MAC_Install
# backup plan for MAC:
# brew install brewsci/bio/pymol

# TODO: Select your profile file to use
PROFILE_FILE="$HOME/.bash_profile"
# PROFILE_FILE="$HOME/.bashrc"

# default shell on newer MAC computers is zsh
# PROFILE_FILE="$HOME/.zshrc" 

# Debian/Ubuntu/Mint
sudo apt-get install git build-essential python3-dev libglew-dev \
  libpng-dev libfreetype6-dev libxml2-dev \
  libmsgpack-dev python3-pyqt5.qtopengl libglm-dev libnetcdf-dev

# for mac, this might not work but
# brew install git pkg-config python3 glew libpng freetype libxml2 msgpack pyqt5 glm netcdf

# Set the path to the PyMOL build directory
PYMOL_BUILD_DIR="$HOME/hax/externals/pymol/build"

# Create the ~/hax/externals directory if it doesn't exist
if [ ! -d "\$HOME/hax/externals" ]; then
    mkdir -p "\$HOME/hax/externals"
fi

# Change to the externals directory
cd "$HOME/hax/externals"

# Create the pymol directory if it doesn't exist
mkdir -p pymol

# Change to the pymol directory
cd pymol

# Clone the PyMOL repository if it doesn't exist
if [ ! -d pymol-open-source ]; then
    gh repo clone schrodinger/pymol-open-source
fi

# Clone the mmtf-cpp repository if it doesn't exist
if [ ! -d mmtf-cpp ]; then
    gh repo clone rcsb/mmtf-cpp
fi

# Move the mmtf-cpp header files to the PyMOL include directory
mv mmtf-cpp/include/mmtf* pymol-open-source/include/

# Change to the PyMOL directory
cd pymol-open-source

# Install PyMOL using the setup.py script
# Set the install command based on the operating system
if [ "$(uname -s)" = "Darwin" ]; then
    INSTALL_CMD="python setup.py install --prefix \"$PYMOL_BUILD_DIR\" --osx-frameworks"
else
    INSTALL_CMD="python setup.py install --prefix \"$PYMOL_BUILD_DIR\""
fi
# Install PyMOL using the install command
eval "$INSTALL_CMD"

# Check if the directory is already included in the profile file
if ! grep -q "$PYMOL_BUILD_DIR/lib/python3.12/site-packages" "$PROFILE_FILE"; then
    # If the directory is not already included, add it to the profile file
    echo "" >> "$PROFILE_FILE"
    echo "if [[ \":\$PYTHONPATH:\" != *\":$PYMOL_BUILD_DIR/lib/python3.12/site-packages:\"* ]]; then" >> "$PROFILE_FILE"
    echo "    export PYTHONPATH=\"$PYMOL_BUILD_DIR/lib/python3.12/site-packages:\$PYTHONPATH\"" >> "$PROFILE_FILE"
    echo "fi" >> "$PROFILE_FILE"
fi

# Source the profile to make the change to PYTHONPATH take effect
source "$PROFILE_FILE"

# Test that PyMOL is installed correctly
python -c "from pymol import cmd, util; print('pymol.cmd:', dir(cmd)); print('pymol.util:', dir(util))"
python -c "import pymol; print('pymol is installed and works on this machine.')"

my bad, removed from rich import print; since others may not have this installed

@JarrettSJohnson JarrettSJohnson changed the title WIP: Adding packaging with pyproject WIP: Packaging with Meson Aug 8, 2024
@ye11owSub
Copy link
Author

@bionicles I would like to achieve this exact result, but at the moment, it is not possible due to a lack of a clear list of dependencies. However, in theory, with the migration to Meson, this goal could be achieved

@ye11owSub ye11owSub closed this Aug 14, 2024
@JarrettSJohnson
Copy link
Member

My major motivation to moving to setuptools was due to distutils being deprecated, but if the larger end-goal is to package via PyPI, that is a goal that's outside of the scope of this repo. We typically do not manage third-party packages and have welcomed the community to do so as long as they maintain it. It's not realistically feasible for us (lack of resources) to maintain open-source package repositories. A couple of examples of pymol-open-source packages in the wild that I've used:

Conda Forge package:
https://github.com/conda-forge/pymol-open-source-feedstock/blob/main/recipe/meta.yaml

Homebrew:
https://github.com/Homebrew/homebrew-core/blob/a975cf07ce0e3ff325ef39e15085f7515daa87ab/Formula/p/pymol.rb

@bionicles
Copy link

I'm confused, you're saying PyMol is "third party" and won't be maintained?

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

Successfully merging this pull request may close these issues.

3 participants