PyAV-Org / PyAV

Pythonic bindings for FFmpeg's libraries.
https://pyav.basswood-io.com/
BSD 3-Clause "New" or "Revised" License
2.39k stars 354 forks source link

Compatibility with NumPy 2.0? #1401

Closed h-vetinari closed 2 months ago

h-vetinari commented 2 months ago

Just wanted to raise some awareness that numpy 2.0.0 will be released soon-ish. There's already an rc2 available on PyPI for testing.

One of the big changes (actually just before numpy 2.0) is that by default, compiling against numpy will now use the oldest-possible numpy ABI for the python versions still supported by numpy. In other words, compiling against 2.0 will produce builds that are (on a C-API level) compatible with numpy >=1.19; of course, if the python side uses newer features, that's a separate constraint.

Since the RC-phase already promises ABI stability, we've already started migrating conda-forge to build against 2.0.0rc2, but for 1.x. The CI in https://github.com/conda-forge/av-feedstock/pull/73 looks green, but we don't test very extensively (mostly some import smoke tests), so I wanted to ask what the status here is.

WyattBlue commented 2 months ago

We don't compile numpy, it's just a run time dependency. I don't see any reason why an older version of PyAV would be incompatible with numpy 2

h-vetinari commented 2 months ago

We don't compile numpy, it's just a run time dependency

You don't compile numpy itself, but you compile (with cython) against the numpy C-API, and are therefore exposed to its ABI. Since the compilation passes fine for 2.0, I guess you're not using any of the features that changed or got removed.

WyattBlue commented 2 months ago

make still works without numpy being install, and 162/270 tests still pass.

h-vetinari commented 2 months ago

I don't see how that's relevant. The question is not whether pyav can work without numpy, but whether it breaks when using numpy 2.0. Could you install python -m pip install --pre numpy==2.0.0rc2 and see if the tests pass then as well?

WyattBlue commented 2 months ago

It means we don't compile numpy in any way, in any stage. I've just tested numpy 2.0.0rc2 locally and all the tests pass.

WyattBlue commented 2 months ago

There is nothing wrong with conda or a distro that chooses to build PyAV with numpy compiled in. I'm just reporting that in the PyPI wheels, it is just a runtime dep, like Pillow.

Numpy 2.0 is officially supported. As it stands, main, and old releases of PyAV should already support numpy 2 out of the box. Feel free to make an issue if that is not the case.

h-vetinari commented 2 months ago

Ok thanks for testing!

distro that chooses to build PyAV with numpy compiled in.

I still think there's a misunderstanding here (or we're not using the same terminology). This repo has code (example) that uses the C-API of numpy. This code can only ever be used through compilation against numpy (with a given version) - in this case, it's done through cython AFAICT (which transpiles to C and can then be compiled normally). So I don't understand where the "just a runtime dep" comes from, unless large swathes of the currently checked-in code here are never used.

WyattBlue commented 2 months ago

Are you talking about _image_fill_pointers_numpy()? I would point to these lines:

        # Using buffer.ctypes.data helps avoid any kind of
        # usage of the c-api from numpy, which avoid the need to add numpy
        # as a compile time dependency
        ...

        c_data = buffer.ctypes.data

The key word being "avoid".

Can you point to the line numbers if you think I'm mistaken?

h-vetinari commented 2 months ago

If you're using numpy buffers in compiled code, you are already depending on the ABI of those buffers. Not only that, you're also converting from your own types while doing so: https://github.com/PyAV-Org/PyAV/blob/b8dc8b98912f3a29a00fa4c00ab23a6a0cdaf624/av/video/frame.pyx#L73

Granted, the ABI of the buffers (and associated functions like frombuffer) is unlikely to change, but I don't see how you can be touching anything numpy-related in compiled (or cythonized) code and then say it's just a run-time dependence. It seems your usage is restricted enough that this doesn't break in practice, but it's not what I would ever consider a runtime dependence.