Skip to content

Conversation

mkoeppe
Copy link
Contributor

@mkoeppe mkoeppe commented May 1, 2023

Description

Please include a short summary of the change.
Issue link (if applicable):

setup_requires is outdated; the build dependencies are declared in pyproject.toml.
Also setuptools should not appear in install_requires; these are the runtime dependencies.

Type of change

  • New feature (backwards compatible)
  • New feature (breaking API changes)
  • Bug fix
  • Other (Documentation, CI, ...)

Contribution checklist

  • Add our license to new files.
  • Check that your code adheres to our coding style.
  • Write unittests.
  • Run the unittests and check that they’re passing.
  • Run the benchmarks to make sure your change doesn’t introduce a regression.

@github-actions
Copy link

github-actions bot commented May 2, 2023

Benchmarks that have stayed the same:

   before           after         ratio
 [ca7d7559]       [c2c5ac81]
      13.5±0s          13.7±0s     1.01  svm_l1_regularization.SVMWithL1Regularization.time_compile_problem
      26.5±0s          26.7±0s     1.01  simple_LP_benchmarks.SimpleLPBenchmark.time_compile_problem
      35.8±0s          35.9±0s     1.00  cvar_benchmark.CVaRBenchmark.time_compile_problem
      1.99±0s          1.99±0s     1.00  simple_LP_benchmarks.SimpleScalarParametrizedLPBenchmark.time_compile_problem
      4.29±0s          4.28±0s     1.00  simple_LP_benchmarks.SimpleFullyParametrizedLPBenchmark.time_compile_problem
      10.0±0s          10.0±0s     1.00  semidefinite_programming.SemidefiniteProgramming.time_compile_problem
      8.18±0s          8.13±0s     0.99  optimal_advertising.OptimalAdvertising.time_compile_problem
      12.4±0s          12.3±0s     0.99  huber_regression.HuberRegression.time_compile_problem
      56.7±0s          54.7±0s     0.96  sdp_segfault_1132_benchmark.SDPSegfault1132Benchmark.time_compile_problem

@SteveDiamond
Copy link
Collaborator

Could you elaborate on how setup_requires is outdated and pyproject.toml is read by python setup.py install? Since what Python version?

@mkoeppe
Copy link
Contributor Author

mkoeppe commented May 2, 2023

It's true that python setup.py install would not look at pyproject.toml.
But python setup.py install -- like all direct invocations of setup.py by users -- has long been deprecated by setuptools. (see https://setuptools.pypa.io/en/latest/history.html#id456, pypa/setuptools#917)

@PTNobel
Copy link
Collaborator

PTNobel commented May 3, 2023

Hey @mkoeppe, I had a few questions that I wasn't understanding about the longer term directions for our setup.py and pyproject.toml; I'm being a bit presumptuous in assuming you're familiar with the python packaging ecosystem, so no worries if you don't have a good answer on what the new best practices are!

  1. Do you think we should eliminate our setup.py eventually? You mentioned it is deprecated (but perhaps I'm misreading you and only ./setup.py install is deprecated). Is removing it compatible with our need to compile a C++ extension on install?
  2. Should we move our dependencies list to pyproject.toml at some point? Is there a benefit in machine readability or some other means?

@PTNobel PTNobel self-requested a review May 3, 2023 06:38
@mkoeppe
Copy link
Contributor Author

mkoeppe commented May 3, 2023

Hi @PTNobel , what is deprecated is calling setup.py directly, with whatever commands (setup.py install, setup.py build) etc. Such commands should no longer be advertised to users.

In modern Python packaging, setuptools is invoked in a different way, namely as a build backend, following PEP 517. (pip and build are frontends.)
Basic distribution packages that just consist of some static files can indeed be built with setuptools without a setup.py, declaring everything in setup.cfg (or, at some point in the future, in pyproject.toml). But this does not apply to distributions that build extension modules etc.

The build backend is declared in pyproject.toml; and there are many options available as alternatives to setuptools. An interesting option is meson-python; the SciPy folks are offering it already as an alternative to their traditional distutils-based build system.

Declaring metadata such as runtime dependencies (install-requires) in pyproject.toml will have the advantage that this will be uniform across all build systems; so it's a first easy step to enable experimentation with other build systems. (That's our plan for SageMath - sagemath/sage#33577)

@PTNobel
Copy link
Collaborator

PTNobel commented May 3, 2023

Thanks for the explanation! (Also I'm a huge SageMath fan! Thanks for all your work there! I've been using it since I was 15!)

I'll have to experiment with the alternative build systems and see if we can find something better for our setup.

@SteveDiamond SteveDiamond merged commit 62ffe67 into cvxpy:master May 3, 2023
Copy link
Collaborator

@phschiele phschiele left a comment

Choose a reason for hiding this comment

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

Thanks for the detailed explanation @mkoeppe!

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.

4 participants