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

Declare build dependency on Cython using pyproject.toml #35

Open
mkoeppe opened this issue Dec 5, 2021 · 7 comments
Open

Declare build dependency on Cython using pyproject.toml #35

mkoeppe opened this issue Dec 5, 2021 · 7 comments
Labels
bug Something isn't working

Comments

@mkoeppe
Copy link

mkoeppe commented Dec 5, 2021

https://snarky.ca/what-the-heck-is-pyproject-toml/

@dave-doty
Copy link
Member

@mkoeppe Did pip install ppsim fail for you? We supposedly have it configured to install cython as a dependency using setup.py, which reads the dependencies from requirements.txt.

I'm curious if it failed, or if you are just suggesting pyproject.toml as a superior alternative to specifying the cython dependency. I hadn't heard of pyproject.toml before. The difference with setup.py seems perhaps to be the "build" step, which is necessary for Linux/Mac systems. (We include a pre-compiled binary for Windows since it is less likely to have the C compiler necessary to build the cython code.)

@mkoeppe
Copy link
Author

mkoeppe commented Dec 5, 2021

The problem with your setup.py is that it cannot even be run if Cython is not already installed because of https://github.com/UC-Davis-molecular-computing/ppsim/blob/main/setup.py#L2
This is a common problem, and is solved in modern Python packaging (PEP 517/518) using pyproject.toml.

Also, declaring cython as an install-requires is likely not needed for your package because it is only needed at build time of the package. You can see this in your wheels -- they are prebuilt but still pull in Cython when they are getting installed.

@dave-doty dave-doty added the bug Something isn't working label Dec 6, 2021
@dave-doty
Copy link
Member

We may also have to do this with numpy, since it is also imported in setup.py. Not sure why; there are some strange path-manipulation lines it is used for:

ppsim/setup.py

Lines 7 to 8 in c29b749

inc_path = np.get_include()
lib_path = join(np.get_include(), '..', '..', 'random', 'lib')

@mkoeppe
Copy link
Author

mkoeppe commented Dec 16, 2021

Yes

@UnHumbleBen
Copy link

@dave-doty @EricESeverson what are the steps/commands used upload to PyPI? I want to try to reproduce the issue in a test package so I can test out the installation after adding the pyproject.toml file and see if the Cython import issue is resolved

@EricESeverson
Copy link
Collaborator

We may also have to do this with numpy, since it is also imported in setup.py. Not sure why; there are some strange path-manipulation lines it is used for:

ppsim/setup.py

Lines 7 to 8 in c29b749

inc_path = np.get_include()
lib_path = join(np.get_include(), '..', '..', 'random', 'lib')

This was some weird wizardry required to get the cython code to be able to directly access the numpy random libraries. I really don't understand how it all works.

@EricESeverson
Copy link
Collaborator

@UnHumbleBen These are the steps I have written down in a text file

change version number
delete files in dist
conda activate base
python setup.py sdist bdist_wheel
conda activate py38
python setup.py sdist bdist_wheel
conda activate py39
python setup.py sdist bdist_wheel
twine upload --repository pypi dist/*
input pypi user information

dave-doty added a commit that referenced this issue Jan 12, 2022
…d-dependency-on-Cython-using-pyproject.toml-#35

Fixes #35; Declare build dependency on Cython using pyproject.toml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants