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

add pyproject.toml file to enable better pip installs #59

Merged
merged 2 commits into from
Apr 7, 2022

Conversation

ahnitz
Copy link
Contributor

@ahnitz ahnitz commented Apr 7, 2022

This adds a boiler plate pyproject.toml file to ensure that pip can install dependencies for the build process of UltraNest. This has largely been pulled from a project I work on (pycbc), but looking at UltraNest's setup.py, it looks like the same set of dependencies are used.

This can tested with

pip install .

pyproject.toml Outdated
"numpy==1.17.3; python_version == '3.8'",
"numpy==1.19.3; python_version == '3.9'",
"numpy==1.21.4; python_version =='3.10'",
"numpy; python_version >= '3.11'",
Copy link

@jpl-jengelke jpl-jengelke Apr 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this breaks anything but there appears to be an extra comma at the end of the line, right before the closing brace for the list.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jpl-jengelke Nope, that doesn't break anything. it's a style convention which allows each line to be the same.

@JohannesBuchner
Copy link
Owner

Hi, sorry to hear that. I had added a pyproject.toml to the project in the meantime, but forgot to git add it. I did this now after seeing issue #56, but before noticing this pull request.

@ahnitz
Copy link
Contributor Author

ahnitz commented Apr 7, 2022

@JohannesBuchner I see the version you had added. I'm not sure that's completely sufficient though as you use numpy within setup.py. Unless the following line can be removed?

https://github.com/JohannesBuchner/UltraNest/blob/master/setup.py#L16

@JohannesBuchner
Copy link
Owner

Is the pyproject.toml with all the versions specified needed? How about using oldest-supported-numpy?

Is setuptools really a requirement?

Why that particular cython version cut?

I also ran into an azure pipeline issue: conda-forge/ultranest-feedstock#35
Can this also be resolved similarly?

@JohannesBuchner
Copy link
Owner

@JohannesBuchner I see the version you had added. I'm not sure that's completely sufficient though as you use numpy within setup.py. Unless the following line can be removed?

https://github.com/JohannesBuchner/UltraNest/blob/master/setup.py#L16

Does the try/catch not catch the ImportError?

@ahnitz
Copy link
Contributor Author

ahnitz commented Apr 7, 2022

@JohannesBuchner Yes, setuptools is really a requirement. There is no guarantee that setuptools is available with python (and in fact that appears to be the common error people often hit). You are right though that oldest-supported-numpy is probably the right thing for ultranest.

@ahnitz
Copy link
Contributor Author

ahnitz commented Apr 7, 2022

@JohannesBuchner I see the version you had added. I'm not sure that's completely sufficient though as you use numpy within setup.py. Unless the following line can be removed?
https://github.com/JohannesBuchner/UltraNest/blob/master/setup.py#L16

Does the try/catch not catch the ImportError?

I just posted our error in #56. It's not the explicit use in setup.py that is the problem.

@ahnitz ahnitz force-pushed the pyproject branch 2 times, most recently from caaf193 to e1cbe05 Compare April 7, 2022 13:43
@ahnitz
Copy link
Contributor Author

ahnitz commented Apr 7, 2022

@JohannesBuchner I see you updated pyproject again, you will also need the "wheel" dependency. See the updated PR here.

@JohannesBuchner
Copy link
Owner

why the wheel dependency? It cannot be compiled as a wheel project, given the cython extensions? Maybe I am confusing something.

@ahnitz
Copy link
Contributor Author

ahnitz commented Apr 7, 2022

@JohannesBuchner Pip installs everything as a wheel now. All that happens when you don't release a wheel binary to pypi is that pip tries to build the wheel locally from the source version.

@ahnitz
Copy link
Contributor Author

ahnitz commented Apr 7, 2022

To be clear, a lot of the time, 'wheel' should already be installed, but like setuptools it's not a hard guarantee if you don't specify it.

@JohannesBuchner
Copy link
Owner

OK, understood.

Can you please change the formatting back to the one I used, which allows removing and adding dependencies with the least number of line changes?

Then I can merge this.

@jpl-jengelke
Copy link

jpl-jengelke commented Apr 7, 2022

Is setuptools really a requirement?

See https://peps.python.org/pep-0518/ which says,

Because the use of setuptools and wheel are so expansive in the community at the moment, build tools are expected to use the example configuration file above as their default semantics when a pyproject.toml file is not present.

The example provided as a baseline is:

[build-system]
# Minimum requirements for the build system to execute.
requires = ["setuptools", "wheel"]  # PEP 508 specifications.

@ahnitz
Copy link
Contributor Author

ahnitz commented Apr 7, 2022

@JohannesBuchner Is it OK now? My editors all autoconvert convert 'tab' to spaces and it seems you had tabs before. If you want to keep your style, probably easier for your you to just add the line and close this PR.

@JohannesBuchner JohannesBuchner merged commit 643e4bb into JohannesBuchner:master Apr 7, 2022
@JohannesBuchner
Copy link
Owner

Thanks a lot!

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