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

adding building with pyproject.toml #375

Conversation

ye11owSub
Copy link
Contributor

@ye11owSub ye11owSub commented Jun 11, 2024

No description provided.

@JarrettSJohnson
Copy link
Member

JarrettSJohnson commented Jul 9, 2024

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: ae5ff74 . 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 ye11owSub changed the title Example: adding building with pyproject and setuptools adding building with pyproject and setuptools Jul 14, 2024
@ye11owSub ye11owSub force-pushed the adding_packaging_with_pyproject_and_setuptools branch 3 times, most recently from 0c55e7f to d7783bf Compare July 14, 2024 23:39
@ye11owSub
Copy link
Contributor Author

ye11owSub commented Jul 14, 2024

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
Copy link
Contributor Author

ye11owSub commented Jul 15, 2024

@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

@ye11owSub ye11owSub changed the title adding building with pyproject and setuptools adding building with pyproject.toml Jul 15, 2024
@JarrettSJohnson
Copy link
Member

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 ye11owSub force-pushed the adding_packaging_with_pyproject_and_setuptools branch from d7783bf to 51b0bc4 Compare August 6, 2024 18:04
@ye11owSub
Copy link
Contributor Author

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
Copy link
Member

JarrettSJohnson commented Aug 6, 2024

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.

setup.py Outdated Show resolved Hide resolved
@ye11owSub ye11owSub force-pushed the adding_packaging_with_pyproject_and_setuptools branch 7 times, most recently from c44c559 to 4d4928b Compare August 8, 2024 00:12
@ye11owSub
Copy link
Contributor Author

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

@ye11owSub ye11owSub force-pushed the adding_packaging_with_pyproject_and_setuptools branch from 4d4928b to ee1ecf8 Compare August 8, 2024 00:24
@JarrettSJohnson JarrettSJohnson self-requested a review August 8, 2024 01:08
@JarrettSJohnson JarrettSJohnson merged commit 757270e into schrodinger:master Aug 8, 2024
3 checks passed
@JarrettSJohnson
Copy link
Member

Thanks for the PR

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