adding building with pyproject.toml

schrodinger / pymol-open-source

Open-source foundation of the user-sponsored PyMOL molecular visualization system.

https://pymol.org/

Other

1.15k stars 275 forks source link

adding building with pyproject.toml #375

Closed ye11owSub closed 1 month ago

JarrettSJohnson commented 2 months ago

Thanks for your patience. Just returned today. I used your pyproject.toml file with a custom backend that forwarded arguments to setup.py. This seems to be a suggested approach from PyPA members; Pillow does this (I pretty much used their approach for this). Commit here: https://github.com/schrodinger/pymol-open-source/commit/ae5ff74657c4d7f247eae6dc17830fe3302db69d . Works for me locally on Windows with pip install --config-settings testing=true . to enable testing as you would before with python setup.py build --testing. I'll work on fixing some tests and CI that I broke along the way, but compiles and runs as expected.

Edit: Tests pass now.

ye11owSub commented 2 months ago

Hi @JarrettSJohnson I hope you had a great vacation! Regarding the PR, I have never tried to use a custom backend, thanks for the advice. So, If this option is suitable now, then it's super! I've added your changes to this PR.

ye11owSub commented 2 months ago

@JarrettSJohnson I decided to test this pip install --config-settings testing=true . command on Ubuntu and macOS. It didn't work as expected Which version of python did you use?

Edit: I figured out what the problem was. Some improvements are needed for the script backend.py

JarrettSJohnson commented 2 months ago

Feel free to also refer to https://github.com/schrodinger/pymol-open-source/actions/runs/9860021376/workflow as it passed with all three operating systems.

ye11owSub commented 1 month ago

Hi @JarrettSJohnson !

Could you please take a look at this PR and let me know if everything looks good to you? Also, I'm having trouble running the CI for this branch(link to CI). Can you please help me out with that?

JarrettSJohnson commented 1 month ago

Accepted the CI run. Seems like for linux it's failing due to missing block:

    - name: Pip upgrade and install build
      run: |
        pip install --upgrade pip
        pip install build

Generally looks good. Though can the functional changes be separated from the formatting changes in setup.py either by commit or PR?

Seems like Github blocks anyone who doesn't have commit history from running CI automatically. Just feel free to ping me so I can manually start it.

ye11owSub commented 1 month ago

@JarrettSJohnson I split the PR into smaller commits and ran the test locally. All should be good now.

JarrettSJohnson commented 1 month ago

Thanks for the PR