Insist on setup file #58
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,20 +14,24 @@ include = [ | |
| ] | ||
|
|
||
| [tool.poetry.dependencies] | ||
| python = ">=3.7,<4.0" | ||
| numpy = [ | ||
| {version = ">=1.20", python = ">=3.7,<3.10"}, | ||
| {version = ">=1.23", python = ">=3.10"} | ||
| ] | ||
| setuptools = ">=51.1.2" | ||
|
|
||
| [tool.poetry.dev-dependencies] | ||
|
Owner
There was a problem hiding this comment. not sure why these need to be changed?
Contributor
Author
There was a problem hiding this comment. I changed setuptools (and other things) to be >= rather than ~ as I feel the upper bound constraints are too restrictive (see https://iscinumpy.dev/post/bound-version-constraints/). For example, the latest version of setuptools is 67.1.0 and the exact-cover package works absolutely fine with that. But the current pyproject.toml insists one downgrades to 51.3.3 or earlier as it is written ~51.1.2 The higher numpy versioning is for easier install on more recent pythons. There are no binary wheels for numpy 1.20 for python 3.10 and up. When poetry does its dependency resolving it fetches an old version of numpy that it has to compile from source (which caused difficulty on the CI windows builds). My change here was just to force the newer python to use newer numpy where binary wheels are available. |
||
| hypothesis = ">=6.56.3" | ||
| pytest = ">=6.1.2" | ||
| flake8 = ">=4.0" | ||
| black = ">=22.3.0" | ||
| valgrindci = ">=0.2.0" | ||
| pytest-fail-slow = ">=0.3.0" | ||
|
|
||
| [tool.poetry.build] | ||
| script = "build.py" | ||
| generate-setup-file = true | ||
|
|
||
| [tool.poetry.scripts] | ||
| test = 'tools.run_tests:test' | ||
|
|
@@ -37,7 +41,7 @@ debug = 'tools.debug:run_debug' | |
| parse_valgrind = 'tools.debug:parse_valgrind_results' | ||
|
|
||
| [build-system] | ||
| requires = ["setuptools", "numpy>=1.20.1", "poetry-core>=1.0.0"] | ||
| build-backend = "poetry.core.masonry.api" | ||
|
|
||
| [tool.black] | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the purpose of this build is to check that the build sdist can correctly be installed.
it would be nice to be sure that both the sdist and the wheel that we distribute can be installed correctly, as the problems you have had were introduced without breaking the build.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this issue is now being picked up by the CI. For example, in your PR #59 from yesterday, the sdist builds are failing with the same error I saw on google colab:
https://github.com/jwg4/exact_cover/actions/runs/4088835028/jobs/7050902881
I think the sdist build problems stem from a breaking change in poetry-core
python-poetry/poetry-core#318
which as of last week's release of poetry-core 1.5.0
https://github.com/python-poetry/poetry-core/releases
no longer generates the setup.py needed for building the binary extension module without us explicitly setting
generate-setup-fileto true (which is what this PR does, and why the tests are passing in this PR but not PR #59).The breaking change to poetry-core was only released a week ago, which is why it's only messing up the CI now. In the comments of the poetry PR, one person comments "I am fairly certain releasing this will break for a few folks building extensions using setuptools, just not sure how large the number is going to be." I think it has indeed broken the exact-cover package, but this PR fixes it.