Fix Cython 3 compatibility #2345

djhoese · 2023-11-28T16:08:45Z

Closes #2268.

This is a first step in Cython 3 compatibility. This is based on a branch started by @takluyver. The basic idea so far is that functions needed to be defined with except X or noexcept to define how errors are handled. The complication was in h5py/h5fd.pyx which casts the h5py version of the file functions (which use a wrapper struct) to match the HDF5 version of the function. Due to what I think is a bug in Cython 3, you can't include an except X statement in your casting so as suggested on the Cython mailing list I created ctypedefs for every function pointer that needed one.

TODO:

Review all performance warnings during compilation. Cython points out that many functions are not handled yet and it is now assuming that exceptions will be raised and the GIL is acquired to check for an exception.
Not needed for initial compatibility, but removal of all IF statements since it is being removed from the Cython language. There are some suggestions on how to not use them, but I'm not sure they will work for all cases used in h5py.

Note the alternative suggested by da-woods (not tagging here to not be annoying) in the above mentioned issue of just specifying the Cython option to force the old behavior.

#2268 (comment)

Example of performance warning

performance hint: h5py/_locks.pxi:114:5: Exception check on 'unlock_lock' will always require the GIL to be acquired.
Possible solutions:
        1. Declare the function as 'noexcept' if you control the definition and you're sure you don't want the function to raise exceptions.
        2. Use an 'int' return type on the function to allow an error code to be returned.

List of performance warnings or similar warnings

performance hint: h5py/defs.pyx:358:33: Exception check will always require the GIL to be acquired.
Possible solutions:
        1. Declare the function as 'noexcept' if you control the definition and you're sure you don't want the function to raise exceptions.
        2. Use an 'int' return type on the function to allow an error code to be returned.
performance hint: h5py/defs.pyx:370:33: Exception check will always require the GIL to be acquired.
Possible solutions:
        1. Declare the function as 'noexcept' if you control the definition and you're sure you don't want the function to raise exceptions.
        2. Use an 'int' return type on the function to allow an error code to be returned.
performance hint: h5py/defs.pyx:479:33: Exception check will always require the GIL to be acquired.
Possible solutions:
        1. Declare the function as 'noexcept' if you control the definition and you're sure you don't want the function to raise exceptions.
        2. Use an 'int' return type on the function to allow an error code to be returned.
performance hint: h5py/defs.pyx:491:33: Exception check will always require the GIL to be acquired.
Possible solutions:
        1. Declare the function as 'noexcept' if you control the definition and you're sure you don't want the function to raise exceptions.
        2. Use an 'int' return type on the function to allow an error code to be returned.
performance hint: h5py/defs.pyx:503:33: Exception check will always require the GIL to be acquired.
Possible solutions:
        1. Declare the function as 'noexcept' if you control the definition and you're sure you don't want the function to raise exceptions.
        2. Use an 'int' return type on the function to allow an error code to be returned.
performance hint: h5py/defs.pyx:515:33: Exception check will always require the GIL to be acquired.
Possible solutions:
        1. Declare the function as 'noexcept' if you control the definition and you're sure you don't want the function to raise exceptions.
        2. Use an 'int' return type on the function to allow an error code to be returned.
performance hint: h5py/defs.pyx:549:33: Exception check will always require the GIL to be acquired.
Possible solutions:
        1. Declare the function as 'noexcept' if you control the definition and you're sure you don't want the function to raise exceptions.
        2. Use an 'int' return type on the function to allow an error code to be returned.
performance hint: h5py/defs.pyx:561:33: Exception check will always require the GIL to be acquired.
Possible solutions:
        1. Declare the function as 'noexcept' if you control the definition and you're sure you don't want the function to raise exceptions.
        2. Use an 'int' return type on the function to allow an error code to be returned.
performance hint: h5py/defs.pyx:573:33: Exception check will always require the GIL to be acquired.
Possible solutions:
        1. Declare the function as 'noexcept' if you control the definition and you're sure you don't want the function to raise exceptions.
        2. Use an 'int' return type on the function to allow an error code to be returned.
performance hint: h5py/defs.pyx:585:33: Exception check will always require the GIL to be acquired.
Possible solutions:
        1. Declare the function as 'noexcept' if you control the definition and you're sure you don't want the function to raise exceptions.
        2. Use an 'int' return type on the function to allow an error code to be returned.
performance hint: h5py/defs.pyx:597:33: Exception check will always require the GIL to be acquired.
Possible solutions:
        1. Declare the function as 'noexcept' if you control the definition and you're sure you don't want the function to raise exceptions.
        2. Use an 'int' return type on the function to allow an error code to be returned.
performance hint: h5py/defs.pyx:609:33: Exception check will always require the GIL to be acquired.
Possible solutions:
        1. Declare the function as 'noexcept' if you control the definition and you're sure you don't want the function to raise exceptions.
        2. Use an 'int' return type on the function to allow an error code to be returned.
performance hint: h5py/defs.pyx:621:33: Exception check will always require the GIL to be acquired.
Possible solutions:
        1. Declare the function as 'noexcept' if you control the definition and you're sure you don't want the function to raise exceptions.
        2. Use an 'int' return type on the function to allow an error code to be returned.
performance hint: h5py/defs.pyx:737:33: Exception check will always require the GIL to be acquired.
Possible solutions:
        1. Declare the function as 'noexcept' if you control the definition and you're sure you don't want the function to raise exceptions.
        2. Use an 'int' return type on the function to allow an error code to be returned.
performance hint: h5py/defs.pyx:1268:33: Exception check will always require the GIL to be acquired.
Possible solutions:
        1. Declare the function as 'noexcept' if you control the definition and you're sure you don't want the function to raise exceptions.
        2. Use an 'int' return type on the function to allow an error code to be returned.
performance hint: h5py/defs.pyx:3833:33: Exception check will always require the GIL to be acquired.
Possible solutions:
        1. Declare the function as 'noexcept' if you control the definition and you're sure you don't want the function to raise exceptions.
        2. Use an 'int' return type on the function to allow an error code to be returned.
performance hint: h5py/defs.pyx:3845:33: Exception check will always require the GIL to be acquired.
Possible solutions:
        1. Declare the function as 'noexcept' if you control the definition and you're sure you don't want the function to raise exceptions.
        2. Use an 'int' return type on the function to allow an error code to be returned.
performance hint: h5py/defs.pyx:3857:33: Exception check will always require the GIL to be acquired.
Possible solutions:
        1. Declare the function as 'noexcept' if you control the definition and you're sure you don't want the function to raise exceptions.
        2. Use an 'int' return type on the function to allow an error code to be returned.

warning: h5py/_errors.pyx:97:84: The keyword 'nogil' should appear at the end of the function signature line. Placing it before 'except' or 'noexcept' will be disallowed in a future version of Cython.

performance hint: h5py/_errors.pyx:171:5: Exception check on 'set_default_error_handler' will always require the GIL to be acquired.
Possible solutions:
        1. Declare the function as 'noexcept' if you control the definition and you're sure you don't want the function to raise exceptions.
        2. Use an 'int' return type on the function to allow an error code to be returned.


performance hint: h5py/_locks.pxi:114:5: Exception check on 'unlock_lock' will always require the GIL to be acquired.
Possible solutions:
        1. Declare the function as 'noexcept' if you control the definition and you're sure you don't want the function to raise exceptions.
        2. Use an 'int' return type on the function to allow an error code to be returned.
[17/24] Cythonizing /home/davidh/repos/git/h5py/h5py/_proxy.pyx



warning: h5py/_proxy.pyx:244:74: The keyword 'nogil' should appear at the end of the function signature line. Placing it before 'except' or 'noexcept' will be disallowed in a future version of Cython.
warning: h5py/_proxy.pyx:255:74: The keyword 'nogil' should appear at the end of the function signature line. Placing it before 'except' or 'noexcept' will be disallowed in a future version of Cython.

djhoese · 2023-11-28T21:13:53Z

Shoot:

Traceback (most recent call last):
  File "h5py/_objects.pyx", line 201, in h5py._objects.ObjectID.__dealloc__
  File "h5py/h5fd.pyx", line 180, in h5py.h5fd.H5FD_fileobj_truncate
AttributeError: 'NoneType' object has no attribute 'truncate'
Exception ignored in: 'h5py._objects.ObjectID.__dealloc__'
Traceback (most recent call last):
  File "h5py/_objects.pyx", line 201, in h5py._objects.ObjectID.__dealloc__
  File "h5py/h5fd.pyx", line 180, in h5py.h5fd.H5FD_fileobj_truncate
AttributeError: 'NoneType' object has no attribute 'truncate'

Not sure what's going on here.

matusvalo · 2023-11-29T09:10:03Z

Shoot:

Traceback (most recent call last):
  File "h5py/_objects.pyx", line 201, in h5py._objects.ObjectID.__dealloc__
  File "h5py/h5fd.pyx", line 180, in h5py.h5fd.H5FD_fileobj_truncate
AttributeError: 'NoneType' object has no attribute 'truncate'
Exception ignored in: 'h5py._objects.ObjectID.__dealloc__'
Traceback (most recent call last):
  File "h5py/_objects.pyx", line 201, in h5py._objects.ObjectID.__dealloc__
  File "h5py/h5fd.pyx", line 180, in h5py.h5fd.H5FD_fileobj_truncate
AttributeError: 'NoneType' object has no attribute 'truncate'

Not sure what's going on here.

Just a note. Maybe you can try to do migration in smaller steps. you can introduce noexcept keywords and all ctypedefs needed without bumping cython version to 3.0.X. Version increase can be done after the CI is green with the change without changing cython version. This is how the migration is done in other projects (e.g. scipy)

takluyver · 2023-11-29T13:07:23Z

Can you reproduce the failure locally? I'd suggest putting the tests into verbose mode (-v option for pytest) to try to narrow down which one is failing, and then see if it still fails when you just run that one. There are some nasty failure modes with the fileobj driver.

djhoese · 2023-11-30T19:59:39Z

Ok so I tested locally and the results are below. The same 2 tests failed but with different failures in one of the tests (race condition maybe). I think the test_register_failure is the same that Azure CI is seeing but something about the tox execution is also causing a seg fault when it fails. I'm running purely pytest h5py/tests/ with a conda-forge based environment on my PopOS (Ubuntu) laptop. Let me try running from master and see what I get.

Cython 3+ local failures

================================================================================================================= FAILURES =================================================================================================================
_____________________________________________________________________________________________________ TestFileObj.test_exception_open ______________________________________________________________________________________________________

self = <h5py.tests.test_file2.TestFileObj testMethod=test_exception_open>

    def test_exception_open(self):
        self.assertRaises(Exception, h5py.File, None,
                          driver='fileobj', mode='x')
>       self.assertRaises(Exception, h5py.File, 'rogue',
                          driver='fileobj', mode='x')
E       AssertionError: Exception not raised by File

h5py/tests/test_file2.py:195: AssertionError
----------------------------------------------------------------------------------------------------------- Captured stderr call -----------------------------------------------------------------------------------------------------------
AttributeError: 'NoneType' object has no attribute 'seek'
___________________________________________________________________________________________________________ test_register_filter ___________________________________________________________________________________________________________

    def test_register_filter():
        filter_id = 256  # Test ID
    
        @H5ZFuncT
        def failing_filter_callback(flags, cd_nelemts, cd_values, nbytes, buf_size, buf):
            return 0
    
        dummy_filter_class = H5ZClass2T(
            version=h5z.CLASS_T_VERS,
            id_=filter_id,
            encoder_present=1,
            decoder_present=1,
            name=b"dummy filter",
            can_apply=None,
            set_local=None,
            filter_=failing_filter_callback,
        )
    
        h5z.register_filter(addressof(dummy_filter_class))
    
        try:
            assert h5z.filter_avail(filter_id)
            filter_flags = h5z.get_filter_info(filter_id)
            assert (
                filter_flags
                == h5z.FILTER_CONFIG_ENCODE_ENABLED | h5z.FILTER_CONFIG_DECODE_ENABLED
            )
        finally:
>           h5z.unregister_filter(filter_id)

h5py/tests/test_h5z.py:75: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
h5py/_objects.pyx:54: in h5py._objects.with_phil.wrapper
    with _phil:
h5py/_objects.pyx:55: in h5py._objects.with_phil.wrapper
    return func(*args, **kwds)
h5py/h5z.pyx:130: in h5py.h5z.unregister_filter
    return <int>H5Zunregister(<H5Z_filter_t>filter_code) >= 0
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

>   (<object>f.fileobj).flush()
E   AttributeError: 'NoneType' object has no attribute 'flush'

h5py/h5fd.pyx:185: AttributeError
============================================================================================================= warnings summary =============================================================================================================
h5py/tests/test_dataset.py::TestCommutative::test_numpy_commutative
  /home/davidh/repos/git/h5py/h5py/tests/test_dataset.py:1944: DeprecationWarning: Conversion of an array with ndim > 0 to a scalar is deprecated, and will error in future. Ensure you extract a single element from your array before performing this operation. (Deprecated NumPy 1.25.)
    val = np.float64(dset[0])

h5py/tests/test_file2.py::TestFileObj::test_exception_open
  /home/davidh/miniconda3/envs/h5py_dev/lib/python3.12/site-packages/_pytest/unraisableexception.py:78: PytestUnraisableExceptionWarning: Exception ignored in: 'h5py.h5fd.H5FD_fileobj_get_eof'

  Traceback (most recent call last):
    File "/home/davidh/repos/git/h5py/h5py/_hl/files.py", line 239, in make_fid
      fid = h5f.create(name, h5f.ACC_EXCL, fapl=fapl, fcpl=fcpl)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  AttributeError: 'NoneType' object has no attribute 'seek'

    warnings.warn(pytest.PytestUnraisableExceptionWarning(msg))

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
========================================================================================================= short test summary info ==========================================================================================================
FAILED h5py/tests/test_file2.py::TestFileObj::test_exception_open - AssertionError: Exception not raised by File
FAILED h5py/tests/test_h5z.py::test_register_filter - AttributeError: 'NoneType' object has no attribute 'flush'
========================================================================================== 2 failed, 724 passed, 12 skipped, 2 warnings in 6.75s ===========================================================================================

Cython <3 local failures

================================================================================================================= FAILURES =================================================================================================================
_____________________________________________________________________________________________________ TestFileObj.test_exception_open ______________________________________________________________________________________________________

self = <h5py.tests.test_file2.TestFileObj testMethod=test_exception_open>

    def test_exception_open(self):
        self.assertRaises(Exception, h5py.File, None,
                          driver='fileobj', mode='x')
>       self.assertRaises(Exception, h5py.File, 'rogue',
                          driver='fileobj', mode='x')
E       AssertionError: Exception not raised by File

h5py/tests/test_file2.py:195: AssertionError
----------------------------------------------------------------------------------------------------------- Captured stderr call -----------------------------------------------------------------------------------------------------------
AttributeError: 'NoneType' object has no attribute 'seek'
___________________________________________________________________________________________________________ test_register_filter ___________________________________________________________________________________________________________

    def test_register_filter():
        filter_id = 256  # Test ID
    
        @H5ZFuncT
        def failing_filter_callback(flags, cd_nelemts, cd_values, nbytes, buf_size, buf):
            return 0
    
        dummy_filter_class = H5ZClass2T(
            version=h5z.CLASS_T_VERS,
            id_=filter_id,
            encoder_present=1,
            decoder_present=1,
            name=b"dummy filter",
            can_apply=None,
            set_local=None,
            filter_=failing_filter_callback,
        )
    
        h5z.register_filter(addressof(dummy_filter_class))
    
        try:
            assert h5z.filter_avail(filter_id)
            filter_flags = h5z.get_filter_info(filter_id)
            assert (
                filter_flags
                == h5z.FILTER_CONFIG_ENCODE_ENABLED | h5z.FILTER_CONFIG_DECODE_ENABLED
            )
        finally:
>           h5z.unregister_filter(filter_id)

h5py/tests/test_h5z.py:75:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
h5py/_objects.pyx:54: in h5py._objects.with_phil.wrapper
    with _phil:
h5py/_objects.pyx:55: in h5py._objects.with_phil.wrapper
    return func(*args, **kwds)
h5py/h5z.pyx:130: in h5py.h5z.unregister_filter
    return <int>H5Zunregister(<H5Z_filter_t>filter_code) >= 0
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

>   (<object>f.fileobj).flush()
E   AttributeError: 'NoneType' object has no attribute 'flush'

h5py/h5fd.pyx:185: AttributeError
============================================================================================================= warnings summary =============================================================================================================
h5py/tests/test_dataset.py::TestCommutative::test_numpy_commutative
  /home/davidh/repos/git/h5py/h5py/tests/test_dataset.py:1944: DeprecationWarning: Conversion of an array with ndim > 0 to a scalar is deprecated, and will error in future. Ensure you extract a single element from your array before performing this operation. (Deprecated NumPy 1.25.)
    val = np.float64(dset[0])

h5py/tests/test_file2.py::TestFileObj::test_exception_open
  /home/davidh/miniconda3/envs/h5py_dev/lib/python3.12/site-packages/_pytest/unraisableexception.py:78: PytestUnraisableExceptionWarning: Exception ignored in: 'h5py.h5fd.H5FD_fileobj_get_eof'

  Traceback (most recent call last):
    File "/home/davidh/repos/git/h5py/h5py/_hl/files.py", line 239, in make_fid
      fid = h5f.create(name, h5f.ACC_EXCL, fapl=fapl, fcpl=fcpl)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  AttributeError: 'NoneType' object has no attribute 'seek'

    warnings.warn(pytest.PytestUnraisableExceptionWarning(msg))

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
========================================================================================================= short test summary info ==========================================================================================================
FAILED h5py/tests/test_file2.py::TestFileObj::test_exception_open - AssertionError: Exception not raised by File
FAILED h5py/tests/test_h5z.py::test_register_filter - AttributeError: 'NoneType' object has no attribute 'flush'
========================================================================================== 2 failed, 724 passed, 12 skipped, 2 warnings in 6.50s ===========================================================================================
Traceback (most recent call last):
  File "h5py/_objects.pyx", line 201, in h5py._objects.ObjectID.__dealloc__
    if H5Idec_ref(self.id) < 0:
  File "h5py/h5fd.pyx", line 180, in h5py.h5fd.H5FD_fileobj_truncate
    (<object>f.fileobj).truncate(f.eoa)
AttributeError: 'NoneType' object has no attribute 'truncate'
Exception ignored in: 'h5py._objects.ObjectID.__dealloc__'
Traceback (most recent call last):
  File "h5py/_objects.pyx", line 201, in h5py._objects.ObjectID.__dealloc__
    if H5Idec_ref(self.id) < 0:
  File "h5py/h5fd.pyx", line 180, in h5py.h5fd.H5FD_fileobj_truncate
    (<object>f.fileobj).truncate(f.eoa)
AttributeError: 'NoneType' object has no attribute 'truncate'

djhoese · 2023-11-30T20:20:01Z

Good news (?), I get the same failures if I add the legacy_implicit_noexcept directive suggested by the Cython maintainer in the referenced issue. So I think that means there aren't any remaining functions that need to have exceptions defined to be valid (although the generate C code should maybe be verified/checked for GIL acquiring).

I'm unable to use this directive with master and building Cython 3 due to what I think is a bug in Cython 3. I'll post on the related issue about that.

takluyver · 2023-12-01T08:43:49Z

That looks like an exception is not passed correctly back through the HDF5 code. In the first example, it should fail to open the file (with the AttributeError you see in captured stderr), but instead it's printing that exception and then apparently 'succeeding' in opening the file, so an HDF5 file ID and an h5py.File object are created. Then I guess the second error might be when garbage collection tries to clear that that up.

`test_exception_open` should get an exception with a traceback like this

In [3]: f = h5py.File("blah", driver='fileobj', mode='x')
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
Cell In[3], line 1
----> 1 f = h5py.File("blah", driver='fileobj', mode='x')

File ~/.local/lib/python3.12/site-packages/h5py/_hl/files.py:562, in File.__init__(self, name, mode, driver, libver, userblock_size, swmr, rdcc_nslots, rdcc_nbytes, rdcc_w0, track_order, fs_strategy, fs_persist, fs_threshold, fs_page_size, page_buf_size, min_meta_keep, min_raw_keep, locking, alignment_threshold, alignment_interval, meta_block_size, **kwds)
    553     fapl = make_fapl(driver, libver, rdcc_nslots, rdcc_nbytes, rdcc_w0,
    554                      locking, page_buf_size, min_meta_keep, min_raw_keep,
    555                      alignment_threshold=alignment_threshold,
    556                      alignment_interval=alignment_interval,
    557                      meta_block_size=meta_block_size,
    558                      **kwds)
    559     fcpl = make_fcpl(track_order=track_order, fs_strategy=fs_strategy,
    560                      fs_persist=fs_persist, fs_threshold=fs_threshold,
    561                      fs_page_size=fs_page_size)
--> 562     fid = make_fid(name, mode, userblock_size, fapl, fcpl, swmr=swmr)
    564 if isinstance(libver, tuple):
    565     self._libver = libver

File ~/.local/lib/python3.12/site-packages/h5py/_hl/files.py:239, in make_fid(name, mode, userblock_size, fapl, fcpl, swmr)
    237     fid = h5f.open(name, h5f.ACC_RDWR, fapl=fapl)
    238 elif mode in ['w-', 'x']:
--> 239     fid = h5f.create(name, h5f.ACC_EXCL, fapl=fapl, fcpl=fcpl)
    240 elif mode == 'w':
    241     fid = h5f.create(name, h5f.ACC_TRUNC, fapl=fapl, fcpl=fcpl)

File h5py/_objects.pyx:54, in h5py._objects.with_phil.wrapper()

File h5py/_objects.pyx:55, in h5py._objects.with_phil.wrapper()

File h5py/h5f.pyx:122, in h5py.h5f.create()

File h5py/h5fd.pyx:155, in h5py.h5fd.H5FD_fileobj_get_eof()

AttributeError: 'NoneType' object has no attribute 'seek'

h5py/h5fd.pyx

djhoese · 2023-12-01T15:50:14Z

Thanks @takluyver. Tests pass locally (Ubuntu) now that get_eof is except -1. Let's see how CI feels.

codecov · 2023-12-01T16:05:39Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (de331ae) 89.53% compared to head (5531827) 89.53%.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #2345   +/-   ##
=======================================
  Coverage   89.53%   89.53%           
=======================================
  Files          17       17           
  Lines        2380     2380           
=======================================
  Hits         2131     2131           
  Misses        249      249

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

djhoese · 2023-12-01T17:15:22Z

CI is all happy. The only other thing I can think of that I've had trouble with in my own projects is GIL access magically being added. However, in my cases this was due to poor error handling. h5py seems like it has that pretty under control and explicit nogil/gil definitions. Let me know if you want me to change anything or discuss something further.

djhoese · 2023-12-01T17:31:46Z

Hm I tried using this branch with my own project's unstable CI (building with Cython 3) and I get a compilation error. This is using HDF5 1.14.2 from conda-forge:

       gcc -pthread -B /usr/share/miniconda3/envs/test-environment/compiler_compat -DNDEBUG -fwrapv -O2 -Wall -fPIC -O2 -isystem /usr/share/miniconda3/envs/test-environment/include -fPIC -O2 -isystem /usr/share/miniconda3/envs/test-environment/include -fPIC -DH5_USE_110_API -DH5Rdereference_vers=2 -DNPY_NO_DEPRECATED_API=0 -Ih5py -I/tmp/pip-req-build-3skh7xs0/lzf -I/tmp/pip-req-build-3skh7xs0/lzf/lzf -I/usr/share/miniconda3/envs/test-environment/lib/python3.11/site-packages/numpy/_core/include -I/usr/share/miniconda3/envs/test-environment/include/python3.11 -c /tmp/pip-req-build-3skh7xs0/h5py/h5t.c -o build/temp.linux-x86_64-cpython-311/tmp/pip-req-build-3skh7xs0/h5py/h5t.o
      In file included from /tmp/pip-req-build-3skh7xs0/h5py/h5t.c:77:
      /tmp/pip-req-build-3skh7xs0/h5py/h5t.c: In function ‘__pyx_f_4h5py_3h5t__c_complex’:
      /tmp/pip-req-build-3skh7xs0/h5py/api_compat.h:38:31: error: request for member ‘real’ in something not a structure or union
         38 | #define h5py_offset_n64_real (HOFFSET(npy_complex64, real))
            |                               ^~~~~~~
      /tmp/pip-req-build-3skh7xs0/h5py/h5t.c:26702:21: note: in expansion of macro ‘h5py_offset_n64_real’
      26702 |     __pyx_v_off_r = h5py_offset_n64_real;
            |                     ^~~~~~~~~~~~~~~~~~~~
      /tmp/pip-req-build-3skh7xs0/h5py/api_compat.h:39:31: error: request for member ‘imag’ in something not a structure or union
         39 | #define h5py_offset_n64_imag (HOFFSET(npy_complex64, imag))
            |                               ^~~~~~~
      /tmp/pip-req-build-3skh7xs0/h5py/h5t.c:26711:21: note: in expansion of macro ‘h5py_offset_n64_imag’
      26711 |     __pyx_v_off_i = h5py_offset_n64_imag;
            |                     ^~~~~~~~~~~~~~~~~~~~
      /tmp/pip-req-build-3skh7xs0/h5py/api_compat.h:40:32: error: request for member ‘real’ in something not a structure or union
         40 | #define h5py_offset_n128_real (HOFFSET(npy_complex128, real))
            |                                ^~~~~~~
      /tmp/pip-req-build-3skh7xs0/h5py/h5t.c:26798:21: note: in expansion of macro ‘h5py_offset_n128_real’
      26798 |     __pyx_v_off_r = h5py_offset_n128_real;
            |                     ^~~~~~~~~~~~~~~~~~~~~
      /tmp/pip-req-build-3skh7xs0/h5py/api_compat.h:41:32: error: request for member ‘imag’ in something not a structure or union
         41 | #define h5py_offset_n128_imag (HOFFSET(npy_complex128, imag))
            |                                ^~~~~~~
      /tmp/pip-req-build-3skh7xs0/h5py/h5t.c:26807:21: note: in expansion of macro ‘h5py_offset_n128_imag’
      26807 |     __pyx_v_off_i = h5py_offset_n128_imag;
            |                     ^~~~~~~~~~~~~~~~~~~~~
      /tmp/pip-req-build-3skh7xs0/h5py/api_compat.h:44:32: error: request for member ‘real’ in something not a structure or union
         44 | #define h5py_offset_n256_real (HOFFSET(npy_complex256, real))
            |                                ^~~~~~~
      /tmp/pip-req-build-3skh7xs0/h5py/h5t.c:26894:21: note: in expansion of macro ‘h5py_offset_n256_real’
      26894 |     __pyx_v_off_r = h5py_offset_n256_real;
            |                     ^~~~~~~~~~~~~~~~~~~~~
      /tmp/pip-req-build-3skh7xs0/h5py/api_compat.h:45:32: error: request for member ‘imag’ in something not a structure or union
         45 | #define h5py_offset_n256_imag (HOFFSET(npy_complex256, imag))
            |                                ^~~~~~~
      /tmp/pip-req-build-3skh7xs0/h5py/h5t.c:26903:21: note: in expansion of macro ‘h5py_offset_n256_imag’
      26903 |     __pyx_v_off_i = h5py_offset_n256_imag;
            |                     ^~~~~~~~~~~~~~~~~~~~~
      error: command '/usr/bin/gcc' failed with exit code 1

Edit: Ah this might be a numpy 2.0 incompatibility.

djhoese · 2023-12-02T02:55:13Z

I'm going to add some changes to fix numpy 2.0 compatibility. I wanted to add a nightly wheel environment, but I'm not a tox expert so not sure how to adapt it. I also see there is an (unused?) nightly environment already. Just in case I'm missing something I didn't want to start.

takluyver · 2023-12-04T16:38:35Z

Thanks! For future reference, I think two things like these (Cython 3.0 & Numpy 2.0 compatibility) are big enough to be two separate PRs, but now that they're both here, let's go with it.

We have testing with pre-release versions already, but Numpy doesn't have a pre-release on PyPI yet. They say that they will have an RC at least 6 weeks before the final release, and I assume that will be on PyPI, so we can test then. That makes more sense to me than testing with nightly builds, which may well mean effort is spent fixing things that are changed again before release.

However, the Numpy 2.0 status issue also says that it will break the C ABI. So for the near future, we're probably going to want to require numpy < 2.0, so that our wheels built against the oldest supported numpy are only installed with a compatible numpy. It looks like there's a plan for a new way to manage compatibility that we can adopt, but we're going to want to be able to test with something close to a final version of Numpy 2.0 before we rely on that.

djhoese · 2023-12-04T16:43:34Z

I think two things like these (Cython 3.0 & Numpy 2.0 compatibility) are big enough to be two separate PRs, but now that they're both here, let's go with it.

Yeah I went back and forth on it. I ended up just being lazy given how small the change ended up being, but yeah understood. Thanks.

For numpy < 2.0, I could be wrong, but I think that would stop a lot of projects from testing h5py with numpy 2.0 in their CI even if it is compatible. Even if you release new wheels with that limit, won't pip still find the old versions which are also not compatible?

takluyver · 2023-12-04T16:59:06Z

I think that would stop a lot of projects from testing h5py with numpy 2.0 in their CI even if it is compatible.

It's still possible for projects to test that, albeit a bit more effort to override h5py's requirement. Hopefully once there's a Numpy RC out, we can figure out what's needed for compatibility and remove the restriction again.

Even if you release new wheels with that limit, won't pip still find the old versions which are also not compatible?

Unfortunately, in many scenarios, yes, that will happen. But I hope making it clear in the metadata for the latest version will help people understand when they manually investigate.

djhoese · 2023-12-04T17:09:38Z

Do you want me to make those changes here?

h5py/_locks.pxi

takluyver · 2023-12-04T17:15:24Z

pyproject.toml

@@ -1,6 +1,6 @@
 [build-system]
 requires = [
-    "Cython >=0.29.31,<1",
+    "Cython >=3",


Does this actually no longer work with Cython 3.0, or did you just change this to ensure it's using a newer version on CI?

I don't think it's a problem if we require a recent version of Cython, but I'd like to check that if it really doesn't work on Cython 0.x any more.

I think it is fine on older Cython's for the syntax used in this PR. I've been doing this change in my own projects to avoid inconsistent builds. I haven't seen any major troubles between different builds beyond some Cython 3 bugs that were fixed. Just a little scared to let it choose whatever. Not a strong opinion on my part as a non-maintainer of this project.

Oh! Maybe this should set a limit (not sure what to) since Cython is deprecating the IF compiler condition (or whatever it is called in Cython-land). I'm not sure if a version has been set when it will actually be removed, but I also know people are still working on the best suggestions for migrating away from it (literal C code, macros, etc).

Good point. Let's put an upper bound of <4 on it, in the hope that actually removing IF would trigger another major version. Last I heard, the warnings about it were being turned off for now while migration paths are worked out, so I don't think removal is imminent 🤞 .

And if the syntax still works on 0.29.x, let's leave the lower bound as it is for now. Although I don't think we actually test building with older Cythons, so it may break without us noticing. 🤷 Testing at the intersection of so many big dependencies (HDF5, Numpy, Python, Cython) is kind of a headache.

And if the syntax still works on 0.29.x, let's leave the lower bound as it is for now

Leave it as it is in master or leave it as it is in this PR? I'll assume 0.29 included.

takluyver · 2023-12-04T17:16:41Z

I've done a separate PR (#2349) to restrict the Numpy requirement for now, so don't worry about that in this PR.

takluyver · 2023-12-05T09:07:06Z

Thanks @djhoese ! 🍪

h5py/h5py#2345 Signed-off-by: wangjiezhe <wangjiezhe@gmail.com>

tacaswell · 2023-12-05T13:07:18Z

Thank you @djhoese !

takluyver and others added 3 commits October 5, 2023 12:44

Fix compiling fileobj file driver with Cython 3.0

b8cc539

Add minimal except definitions for Cython 3 compatibility

c0a7b98

Force Cython to version 3 during build

8a1c2ed

djhoese mentioned this pull request Nov 28, 2023

build issue when using cython 3.0 #2268

Closed

djhoese added 2 commits November 28, 2023 14:12

Update remaining except/nogil cython 3 issues

2f7c051

Merge branch 'master' into cython-3-davidh

5413d58

djhoese marked this pull request as ready for review November 28, 2023 20:13

takluyver reviewed Dec 1, 2023

View reviewed changes

h5py/h5fd.pyx Outdated Show resolved Hide resolved

Fix get_eof exception definition

d6b267d

djhoese mentioned this pull request Dec 1, 2023

Numpy 2.x incompatibilities #2348

Closed

djhoese added 2 commits December 1, 2023 21:09

Fix numpy warning about array to scalar conversion

f0aa23d

Fix compatibility with numpy 2

51d3c3a

takluyver mentioned this pull request Dec 4, 2023

Require NumPy < 2 (for now) #2349

Closed

takluyver reviewed Dec 4, 2023

View reviewed changes

h5py/_locks.pxi Show resolved Hide resolved

takluyver reviewed Dec 4, 2023

View reviewed changes

djhoese added 2 commits December 4, 2023 13:52

Add explicit noexcept to lock functions

502be17

Update Cython version limits to include 0.29 and up to 4

5531827

takluyver merged commit 4c01efa into h5py:master Dec 5, 2023
39 checks passed

wangjiezhe added a commit to wangjiezhe/gentoo-local that referenced this pull request Dec 5, 2023

dev-python/h5py: use official patch for Cython 3

84315ef

h5py/h5py#2345 Signed-off-by: wangjiezhe <wangjiezhe@gmail.com>

larsoner mentioned this pull request Dec 11, 2023

Numpy 2.0 compat plan #2353

Closed

takluyver added this to the 3.11 milestone Apr 5, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix Cython 3 compatibility #2345

Fix Cython 3 compatibility #2345

djhoese commented Nov 28, 2023 •

edited

djhoese commented Nov 28, 2023

matusvalo commented Nov 29, 2023

takluyver commented Nov 29, 2023

djhoese commented Nov 30, 2023

djhoese commented Nov 30, 2023

takluyver commented Dec 1, 2023

djhoese commented Dec 1, 2023

codecov bot commented Dec 1, 2023 •

edited

djhoese commented Dec 1, 2023

djhoese commented Dec 1, 2023 •

edited

djhoese commented Dec 2, 2023 •

edited

takluyver commented Dec 4, 2023

djhoese commented Dec 4, 2023

takluyver commented Dec 4, 2023

djhoese commented Dec 4, 2023

takluyver Dec 4, 2023

djhoese Dec 4, 2023

djhoese Dec 4, 2023

takluyver Dec 4, 2023

djhoese Dec 4, 2023

takluyver commented Dec 4, 2023

takluyver commented Dec 5, 2023

tacaswell commented Dec 5, 2023

Fix Cython 3 compatibility #2345

Fix Cython 3 compatibility #2345

Conversation

djhoese commented Nov 28, 2023 • edited

Example of performance warning

djhoese commented Nov 28, 2023

matusvalo commented Nov 29, 2023

takluyver commented Nov 29, 2023

djhoese commented Nov 30, 2023

djhoese commented Nov 30, 2023

takluyver commented Dec 1, 2023

djhoese commented Dec 1, 2023

codecov bot commented Dec 1, 2023 • edited

Codecov Report

djhoese commented Dec 1, 2023

djhoese commented Dec 1, 2023 • edited

djhoese commented Dec 2, 2023 • edited

takluyver commented Dec 4, 2023

djhoese commented Dec 4, 2023

takluyver commented Dec 4, 2023

djhoese commented Dec 4, 2023

takluyver Dec 4, 2023

Choose a reason for hiding this comment

djhoese Dec 4, 2023

Choose a reason for hiding this comment

djhoese Dec 4, 2023

Choose a reason for hiding this comment

takluyver Dec 4, 2023

Choose a reason for hiding this comment

djhoese Dec 4, 2023

Choose a reason for hiding this comment

takluyver commented Dec 4, 2023

takluyver commented Dec 5, 2023

tacaswell commented Dec 5, 2023

djhoese commented Nov 28, 2023 •

edited

codecov bot commented Dec 1, 2023 •

edited

djhoese commented Dec 1, 2023 •

edited

djhoese commented Dec 2, 2023 •

edited