search

kleinerm / Psychtoolbox-3

This is kleinerm's git repository for development of Psychtoolbox-3. Regular end users should stay away from it, unless instructed by him otherwise, and use the official Psychtoolbox-3 GitHub page or distribution system for production releases.
104 stars 306 forks source link

Fixes for NumPy 2.0 #276

Closed larsoner closed 3 months ago

larsoner commented 3 months ago
  1. Add a content type for the description in setup.py, without it twine check --strict complains.
  2. Build against NumPy 2.0 in the build-system when possible (i.e., on Python 3.9 or later). This will create code that is compatible with both NumPy 1.x and 2.x.
  3. Replace PyArray_REFCOUNT with the drop-in replacement Py_REFCNT (the former is gone in NumPy 2.0)

The first commit is the content-type fix. The next commit contains the two changes that make things compatible with NumPy 2.0, done following suggestions in their release notes.

I've tested that the wheels produced using this branch work on Linux as well in https://github.com/aforren1/ptb-wheels/pull/10.

kleinerm commented 3 months ago

This looks fine to me, apart from commit cosmetics - code itself seems fine:

  1. Maybe split into 3 commits, as all three are independent changes.
  2. Commit 2: Subject line maybe PsychPythonGlue: ... instead of PythonGlue: ...
  3. The commits need commit messages explaining why the changes were made and why they are safe and sufficiently backwards compatible. Basically what you have in your pull request description above, but split up to the different commits. More verbose is better than less verbose in commit messages.

Wrt. NumPy 2: I see that Ubuntu 22.04 ships with Python 3.10, but I can't see NumPy 2 in the repository. I assume I could still test build locally if I wanted to? Haven't test rebuild the Python version in years.

larsoner commented 3 months ago

This looks fine to me, apart from commit cosmetics

Okay force-pushed three more verbose commits, let me know if these are good enough or need further tweaks!

Wrt. NumPy 2: I see that Ubuntu 22.04 ships with Python 3.10, but I can't see NumPy 2 in the repository. I assume I could still test build locally if I wanted to? Haven't test rebuild the Python version in years.

Yep you can! The nice thing about all this build-system stuff in pyproject.toml is that it doesn't really matter what NumPy you actually have installed on your system, or what Ubuntu 22.04 ships with, etc. pip nowadays uses the build-system specs to create an isolated build environment (which will have NumPy 2.0 after this PR, at least on Python 3.9+), build the wheel, and then install that back in the original environment. And the NumPy folks made sure that code built with NumPy 2.0 is backward compatible with NumPy 1.x installs, so it should all work okay even if you have NumPy 1.x installed.

kleinerm commented 3 months ago

Better. Nitpick: The commit summary line ideally should have proper capitalization, ie, "Set the description..." and "Prefer NumPy..." and end with a . - Like a mostly proper sentence. Similar in the commit text for commit 2, a properly terminated sentence. And it would be good to add the links to those NumPy 2 release notes and the testing with Alex's wheels. Basically what is now in your pull request message but not in the commit messages. You almost can't be to verbose on the PTB side.

larsoner commented 3 months ago

Thanks -- done!

kleinerm commented 3 months ago

Perfect, thanks.