-
Notifications
You must be signed in to change notification settings - Fork 1
Update Build Toolchain #53
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
Conversation
0d64146 to
79aebce
Compare
6144a57 to
92b6482
Compare
cbd1543 to
9bf5d57
Compare
7b3d070 to
ea4baf6
Compare
|
In the process of getting To fix this, I decided to follow PyPA's recommendation to have all package source files inside a Also, I added a temporary job to |
* Fix the clunky CLI test runner. * Update the README about it. * The `--plot` test requires installing matplotlib, in particular, one that's compatible with the installed numpy. * Moved the `analysis/` directory into the `test/` directory for the tests to import. * TODO: Fix the `--memory` test, which fails `assert memory_increases <= 10` on macOS. * TODO: Fix the `--plot` test, which raises `ValueError: x and y must have same first dimension, but have shapes (115,) and (114, 2469)` from matplotlib. * Delete `release.sh` which would be risky to run now and has been supplanted by GitHub Actions. Actually my process was to copy/paste from this script rather than run it as is. Q. Do *.pxd header files need `freethreading_compatible = True` or is it sufficient in the .pyx module?
1fish2
left a comment
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.
This looks great!
I fixed the CLI test runner although bugs remain in the --plot and --memory tests. See the commit message.
|
Thanks for the review and edits @1fish2! I noticed that the memory test in its current form fails on MacOS and changed it to |
prismofeverything
left a comment
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.
Huge improvement all around, thanks for bringing arrow into the modern age (!) This pattern is very useful - if it were worthwhile this could be the basis of a cython template that could be reused in the future (just had to go through a similar thing recently).
|
I granted trusted publisher access to this repository for the package on pypi in the |
This removes the deprecated
distutilsin favor ofsetuptoolsand replaces thesetup.pyCLI withpipandbuild.Here is a table of environments where
python stochastic_arrow/test/test_arrow.pyran without issue after installing either ansdist(platform-agnostic) orwheel(platform-specific) that I built on macOS 15.1.1, Python 3.12, Cython 3.0.11, and Numpy 2.1.3 withpython -m build --sdist/wheel:debian:latestsdistdebian:latestsdistsdistwheelIt's pretty cool that this can still be installed on Python 2.7 with just a few tweaks. The last row suggests that building with Numpy 2.1.3 does not make the wheel incompatible with older versions of Numpy.
Also, I added a
cibuildwheelGitHub Action that should automatically build and publish wheels for all common platforms and recent Python versions. It should runpyteston every wheel it builds so we won't accidentally upload a non-functional wheel.I got the wheel working on the new free-threaded release of Python 3.13. Unfortunately, it was almost universally slower than the normal Python 3.13 both to build and to run
pytest.