DRAFT: Updated packaging scheme #147
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Put all information needed by pip to build distributions in a dedicated build system venv in the TOML file. All information needed to dictate the composition of the package in setup.py. I was able to remove dependencies.
With this, I can get tox -e coverage working. I see that .so is installed correctly in the nocoverage installation as well.
This is recommended by both the Cython and setuptools docs. As a result Cython need only be installed by developers that are actually working on that file. setup.py is responsible only for compiling the c file, which requires numpy headers but not cython. This can be seen in the changes here including the fact that the test suite no longer needs to cythonize the c file to run. I added a cythonize tox task to setup a venv with Cython and generate the c file with it. The coverage test is still passing. I was also able to install surmise in a venv and run the GP_surmise notebook sucessfully, which should be using the Cython code.
I have never had to exclude files like this. If I comment out the Extension line in setup.py and get rid of pyproject.toml, I don't in fact need to do the exclusions. Note clear why the TOML file is causing this mass inclusion by default.
This is necessitated so that binary wheels are built with a numpy version that is ABI compatible with all numpy versions that users can choose to install alongside surmise.
The inability that I had with installing numpy < 1.26 in build systems is no longer an issue since we are now fixed at the more modern 2.0.0. On my system, I was able to get tests working with 1.22.0. Developers should all use the same Cython version. Explicitly fix the Cython language directive to Python 3 to get rid of warnings. According to the warning, it was previously using 3str by default. Therefore, there are a few changes to the .c file, which was generated using the new Cython version and the target 2.0.0 numpy version.
The actions also failed on macOS-13, which was using numpy 2.2.2. Other macos tests passed with older numpy versions. I did see at least one Ubuntu test pass with 2.2.2. Try restricting version to see if we can get all tests passing.
Try numpy 2.0.0 to match the version used to build wheels. Also try adding in Python 3.13.
Remove macos-13 and see if any others are failing with the newest numpy.
macos-13 works in the oldest test with Python 3.9. See if it works with more modern numpy.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I need a PR to get the actions running.