JohannesBuchner / UltraNest

Fit and compare complex models reliably and rapidly. Advanced nested sampling.
https://johannesbuchner.github.io/UltraNest/
Other
142 stars 30 forks source link

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

Closed ahnitz closed 2 years ago

ahnitz commented 2 years ago

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 .
JohannesBuchner commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

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: https://github.com/conda-forge/ultranest-feedstock/pull/35 Can this also be resolved similarly?

JohannesBuchner commented 2 years ago

@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 commented 2 years ago

@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 commented 2 years ago

@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 commented 2 years ago

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

JohannesBuchner commented 2 years ago

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

ahnitz commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

Thanks a lot!