search

LLNL / zfp

Compressed numerical arrays that support high-speed random access
http://zfp.llnl.gov
BSD 3-Clause "New" or "Revised" License
768 stars 155 forks source link

fix: syntax error in setup.py + modernize #236

Closed william-silversmith closed 3 months ago

william-silversmith commented 3 months ago

Fixes the syntax error, cythonizes dynamically (may make this run on more platforms), sets language level to 3 (Python 2 is EOL for years now) to remove a warning, and adds license and classifiers so PyPI can categorize it better.

Loads numpy dynamically so it won't crash if numpy was installed at the same time as the setup call is run.

Uses numpy for installation, but oldest-supported-numpy for isolated compilation for py38 via pyproject.toml

Adds MANIFEST.in to ensure sdist includes source files, LICENSE, etc.

Resolves #233

lindstro commented 3 months ago

@william-silversmith Thanks, this looks great. But no PR should target the master branch, which is the latest official release. Please target develop instead.

Also, there's been a lot of activity lately around NumPy, zfpy in general, and the setup.py bug referenced here. See #234, #233, #231, #227, #210, #201. It would be good to know to what extent there's overlap between these PRs and issues. I'm unfortunately not well versed in Python, so it would be good for those with more expertise to come to a consensus as to what changes are needed. Part of this includes how to best deal with failing AppVeyor tests (some because of lack of NumPy 2.0, some because CMake is too old). I'm too committed with other zfp work at the moment to figure out if now is the time to move our Windows tests to GitHub Actions, which presumably would solve the issues we're having with AppVeyor.

william-silversmith commented 3 months ago

Sorry about that, I'll close this and make a new PR on develop. For myself, I can comment on other issues, but for Windows, I don't have regular access to a Windows machine, so that is more difficult.