Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix Cython 3 compatibility #2345

Merged
merged 10 commits into from
Dec 5, 2023
+68 −53
Merged

Fix Cython 3 compatibility #2345

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
b8cc539
Fix compiling fileobj file driver with Cython 3.0
takluyver Oct 5, 2023
c0a7b98
Add minimal except definitions for Cython 3 compatibility
djhoese Nov 28, 2023
8a1c2ed
Force Cython to version 3 during build
djhoese Nov 28, 2023
2f7c051
Update remaining except/nogil cython 3 issues
djhoese Nov 28, 2023
5413d58
Merge branch 'master' into cython-3-davidh
djhoese Nov 28, 2023
d6b267d
Fix get_eof exception definition
djhoese Dec 1, 2023
f0aa23d
Fix numpy warning about array to scalar conversion
djhoese Dec 2, 2023
51d3c3a
Fix compatibility with numpy 2
djhoese Dec 2, 2023
502be17
Add explicit noexcept to lock functions
djhoese Dec 4, 2023
5531827
Update Cython version limits to include 0.29 and up to 4
djhoese Dec 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion h5py/_errors.pxd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -425,4 +425,4 @@ ctypedef struct err_cookie:
cdef err_cookie set_error_handler(err_cookie handler)

# Set the default error handler set by silence_errors/unsilence_errors
cdef void set_default_error_handler() nogil
cdef void set_default_error_handler() noexcept nogil
4 changes: 2 additions & 2 deletions h5py/_errors.pyx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ cdef struct err_data_t:
H5E_error_t err
int n

cdef herr_t walk_cb(unsigned int n, const H5E_error_t *desc, void *e) nogil noexcept:
cdef herr_t walk_cb(unsigned int n, const H5E_error_t *desc, void *e) noexcept nogil:

cdef err_data_t *ee = <err_data_t*>e

Expand Down Expand Up @@ -168,7 +168,7 @@ cdef err_cookie _error_handler # Store error handler used by h5py
_error_handler.func = NULL
_error_handler.data = NULL

cdef void set_default_error_handler() nogil:
cdef void set_default_error_handler() noexcept nogil:
"""Set h5py's current default error handler"""
H5Eset_auto(<hid_t>H5E_DEFAULT, _error_handler.func, _error_handler.data)

Expand Down
2 changes: 1 addition & 1 deletion h5py/_locks.pxi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ cdef bint _acquire_lock(FastRLock lock, long current_thread, int wait) nogil:
lock._count = 1
return 1

cdef inline void unlock_lock(FastRLock lock) nogil:
cdef inline void unlock_lock(FastRLock lock) noexcept nogil:
takluyver marked this conversation as resolved.
Show resolved Hide resolved
# Note that this function *must* hold the GIL when being called.
# We just use 'nogil' in the signature to make sure that no Python
# code execution slips in that might free the GIL
Expand Down
4 changes: 2 additions & 2 deletions h5py/_proxy.pyx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ ctypedef struct h5py_scatter_t:
void* buf

cdef herr_t h5py_scatter_cb(void* elem, hid_t type_id, unsigned ndim,
const hsize_t *point, void *operator_data) nogil except -1:
const hsize_t *point, void *operator_data) except -1 nogil:
cdef h5py_scatter_t* info = <h5py_scatter_t*>operator_data

memcpy(elem, (<char*>info[0].buf)+((info[0].i)*(info[0].elsize)),
Expand All @@ -252,7 +252,7 @@ cdef herr_t h5py_scatter_cb(void* elem, hid_t type_id, unsigned ndim,
return 0

cdef herr_t h5py_gather_cb(void* elem, hid_t type_id, unsigned ndim,
const hsize_t *point, void *operator_data) nogil except -1:
const hsize_t *point, void *operator_data) except -1 nogil:
cdef h5py_scatter_t* info = <h5py_scatter_t*>operator_data

memcpy((<char*>info[0].buf)+((info[0].i)*(info[0].elsize)), elem,
Expand Down
48 changes: 24 additions & 24 deletions h5py/api_types_hdf5.pxd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,27 +257,27 @@ cdef extern from "hdf5.h":
herr_t (*sb_encode)(H5FD_t *file, char *name, unsigned char *p)
herr_t (*sb_decode)(H5FD_t *f, const char *name, const unsigned char *p)
size_t fapl_size
void * (*fapl_get)(H5FD_t *file)
void * (*fapl_copy)(const void *fapl)
herr_t (*fapl_free)(void *fapl)
void * (*fapl_get)(H5FD_t *file) except *
void * (*fapl_copy)(const void *fapl) except *
herr_t (*fapl_free)(void *fapl) except -1
size_t dxpl_size
void * (*dxpl_copy)(const void *dxpl)
herr_t (*dxpl_free)(void *dxpl)
H5FD_t *(*open)(const char *name, unsigned flags, hid_t fapl, haddr_t maxaddr)
herr_t (*close)(H5FD_t *file)
H5FD_t *(*open)(const char *name, unsigned flags, hid_t fapl, haddr_t maxaddr) except *
herr_t (*close)(H5FD_t *file) except -1
int (*cmp)(const H5FD_t *f1, const H5FD_t *f2)
herr_t (*query)(const H5FD_t *f1, unsigned long *flags)
herr_t (*get_type_map)(const H5FD_t *file, H5FD_mem_t *type_map)
haddr_t (*alloc)(H5FD_t *file, H5FD_mem_t type, hid_t dxpl_id, hsize_t size)
herr_t (*free)(H5FD_t *file, H5FD_mem_t type, hid_t dxpl_id, haddr_t addr, hsize_t size)
haddr_t (*get_eoa)(const H5FD_t *file, H5FD_mem_t type)
herr_t (*set_eoa)(H5FD_t *file, H5FD_mem_t type, haddr_t addr)
haddr_t (*get_eof)(const H5FD_t *file, H5FD_mem_t type)
haddr_t (*get_eoa)(const H5FD_t *file, H5FD_mem_t type) noexcept
herr_t (*set_eoa)(H5FD_t *file, H5FD_mem_t type, haddr_t addr) noexcept
haddr_t (*get_eof)(const H5FD_t *file, H5FD_mem_t type) noexcept
herr_t (*get_handle)(H5FD_t *file, hid_t fapl, void**file_handle)
herr_t (*read)(H5FD_t *file, H5FD_mem_t type, hid_t dxpl, haddr_t addr, size_t size, void *buffer)
herr_t (*write)(H5FD_t *file, H5FD_mem_t type, hid_t dxpl, haddr_t addr, size_t size, const void *buffer)
herr_t (*flush)(H5FD_t *file, hid_t dxpl_id, hbool_t closing)
herr_t (*truncate)(H5FD_t *file, hid_t dxpl_id, hbool_t closing)
herr_t (*read)(H5FD_t *file, H5FD_mem_t type, hid_t dxpl, haddr_t addr, size_t size, void *buffer) except *
herr_t (*write)(H5FD_t *file, H5FD_mem_t type, hid_t dxpl, haddr_t addr, size_t size, const void *buffer) except *
herr_t (*flush)(H5FD_t *file, hid_t dxpl_id, hbool_t closing) except -1
herr_t (*truncate)(H5FD_t *file, hid_t dxpl_id, hbool_t closing) except -1
herr_t (*lock)(H5FD_t *file, hbool_t rw)
herr_t (*unlock)(H5FD_t *file)
H5FD_mem_t fl_map[<int>H5FD_MEM_NTYPES]
Expand All @@ -295,27 +295,27 @@ cdef extern from "hdf5.h":
herr_t (*sb_encode)(H5FD_t *file, char *name, unsigned char *p)
herr_t (*sb_decode)(H5FD_t *f, const char *name, const unsigned char *p)
size_t fapl_size
void * (*fapl_get)(H5FD_t *file)
void * (*fapl_copy)(const void *fapl)
herr_t (*fapl_free)(void *fapl)
void * (*fapl_get)(H5FD_t *file) except *
void * (*fapl_copy)(const void *fapl) except *
herr_t (*fapl_free)(void *fapl) except -1
size_t dxpl_size
void * (*dxpl_copy)(const void *dxpl)
herr_t (*dxpl_free)(void *dxpl)
H5FD_t *(*open)(const char *name, unsigned flags, hid_t fapl, haddr_t maxaddr)
herr_t (*close)(H5FD_t *file)
H5FD_t *(*open)(const char *name, unsigned flags, hid_t fapl, haddr_t maxaddr) except *
herr_t (*close)(H5FD_t *file) except -1
int (*cmp)(const H5FD_t *f1, const H5FD_t *f2)
herr_t (*query)(const H5FD_t *f1, unsigned long *flags)
herr_t (*get_type_map)(const H5FD_t *file, H5FD_mem_t *type_map)
haddr_t (*alloc)(H5FD_t *file, H5FD_mem_t type, hid_t dxpl_id, hsize_t size)
herr_t (*free)(H5FD_t *file, H5FD_mem_t type, hid_t dxpl_id, haddr_t addr, hsize_t size)
haddr_t (*get_eoa)(const H5FD_t *file, H5FD_mem_t type)
herr_t (*set_eoa)(H5FD_t *file, H5FD_mem_t type, haddr_t addr)
haddr_t (*get_eof)(const H5FD_t *file, H5FD_mem_t type)
haddr_t (*get_eoa)(const H5FD_t *file, H5FD_mem_t type) noexcept
herr_t (*set_eoa)(H5FD_t *file, H5FD_mem_t type, haddr_t addr) noexcept
haddr_t (*get_eof)(const H5FD_t *file, H5FD_mem_t type) noexcept
herr_t (*get_handle)(H5FD_t *file, hid_t fapl, void**file_handle)
herr_t (*read)(H5FD_t *file, H5FD_mem_t type, hid_t dxpl, haddr_t addr, size_t size, void *buffer)
herr_t (*write)(H5FD_t *file, H5FD_mem_t type, hid_t dxpl, haddr_t addr, size_t size, const void *buffer)
herr_t (*flush)(H5FD_t *file, hid_t dxpl_id, hbool_t closing)
herr_t (*truncate)(H5FD_t *file, hid_t dxpl_id, hbool_t closing)
herr_t (*read)(H5FD_t *file, H5FD_mem_t type, hid_t dxpl, haddr_t addr, size_t size, void *buffer) except *
herr_t (*write)(H5FD_t *file, H5FD_mem_t type, hid_t dxpl, haddr_t addr, size_t size, const void *buffer) except *
herr_t (*flush)(H5FD_t *file, hid_t dxpl_id, hbool_t closing) except -1
herr_t (*truncate)(H5FD_t *file, hid_t dxpl_id, hbool_t closing) except -1
herr_t (*lock)(H5FD_t *file, hbool_t rw)
herr_t (*unlock)(H5FD_t *file)
H5FD_mem_t fl_map[<int>H5FD_MEM_NTYPES]
Expand Down
40 changes: 28 additions & 12 deletions h5py/h5fd.pyx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,14 +144,14 @@ cdef herr_t H5FD_fileobj_close(H5FD_fileobj_t *f) except -1 with gil:
stdlib_free(f)
return 0

cdef haddr_t H5FD_fileobj_get_eoa(const H5FD_fileobj_t *f, H5FD_mem_t type):
cdef haddr_t H5FD_fileobj_get_eoa(const H5FD_fileobj_t *f, H5FD_mem_t type) noexcept nogil:
return f.eoa

cdef herr_t H5FD_fileobj_set_eoa(H5FD_fileobj_t *f, H5FD_mem_t type, haddr_t addr):
cdef herr_t H5FD_fileobj_set_eoa(H5FD_fileobj_t *f, H5FD_mem_t type, haddr_t addr) noexcept nogil:
f.eoa = addr
return 0

cdef haddr_t H5FD_fileobj_get_eof(const H5FD_fileobj_t *f, H5FD_mem_t type) except -1 with gil: # HADDR_UNDEF
cdef haddr_t H5FD_fileobj_get_eof(const H5FD_fileobj_t *f, H5FD_mem_t type) noexcept with gil: # HADDR_UNDEF
(<object>f.fileobj).seek(0, libc.stdio.SEEK_END)
return (<object>f.fileobj).tell()
djhoese marked this conversation as resolved.
Show resolved Hide resolved

Expand Down Expand Up @@ -191,22 +191,38 @@ cdef herr_t H5FD_fileobj_flush(H5FD_fileobj_t *f, hid_t dxpl, hbool_t closing) e
cdef H5FD_class_t info
memset(&info, 0, sizeof(info))

# Cython doesn't support "except X" in casting definition currently
ctypedef herr_t (*file_free_func_ptr)(void *) except -1

ctypedef herr_t (*file_close_func_ptr)(H5FD_t *) except -1
ctypedef haddr_t (*file_get_eoa_func_ptr)(const H5FD_t *, H5FD_mem_t) noexcept
ctypedef herr_t (*file_set_eof_func_ptr)(H5FD_t *, H5FD_mem_t, haddr_t) noexcept
ctypedef haddr_t (*file_get_eof_func_ptr)(const H5FD_t *, H5FD_mem_t) noexcept
ctypedef herr_t (*file_read_func_ptr)(H5FD_t *, H5FD_mem_t, hid_t, haddr_t, size_t, void*) except -1
ctypedef herr_t (*file_write_func_ptr)(H5FD_t *, H5FD_mem_t, hid_t, haddr_t, size_t, const void*) except -1
ctypedef herr_t (*file_truncate_func_ptr)(H5FD_t *, hid_t, hbool_t) except -1
ctypedef herr_t (*file_flush_func_ptr)(H5FD_t *, hid_t, hbool_t) except -1


info.name = 'fileobj'
info.maxaddr = libc.stdint.SIZE_MAX - 1
info.fc_degree = H5F_CLOSE_WEAK
info.fapl_size = sizeof(PyObject *)
info.fapl_get = <void *(*)(H5FD_t *)>H5FD_fileobj_fapl_get
info.fapl_copy = <void *(*)(const void *)>H5FD_fileobj_fapl_copy
info.fapl_free = <herr_t (*)(void *)>H5FD_fileobj_fapl_free

info.fapl_free = <file_free_func_ptr>H5FD_fileobj_fapl_free

info.open = <H5FD_t *(*)(const char *name, unsigned flags, hid_t fapl, haddr_t maxaddr)>H5FD_fileobj_open
info.close = <herr_t (*)(H5FD_t *)>H5FD_fileobj_close
info.get_eoa = <haddr_t (*)(const H5FD_t *, H5FD_mem_t)>H5FD_fileobj_get_eoa
info.set_eoa = <herr_t (*)(H5FD_t *, H5FD_mem_t, haddr_t)>H5FD_fileobj_set_eoa
info.get_eof = <haddr_t (*)(const H5FD_t *, H5FD_mem_t)>H5FD_fileobj_get_eof
info.read = <herr_t (*)(H5FD_t *, H5FD_mem_t, hid_t, haddr_t, size_t, void *)>H5FD_fileobj_read
info.write = <herr_t (*)(H5FD_t *, H5FD_mem_t, hid_t, haddr_t, size_t, const void *)>H5FD_fileobj_write
info.truncate = <herr_t (*)(H5FD_t *, hid_t, hbool_t)>H5FD_fileobj_truncate
info.flush = <herr_t (*)(H5FD_t *, hid_t, hbool_t)>H5FD_fileobj_flush

info.close = <file_close_func_ptr>H5FD_fileobj_close
info.get_eoa = <file_get_eoa_func_ptr>H5FD_fileobj_get_eoa
info.set_eoa = <file_set_eof_func_ptr>H5FD_fileobj_set_eoa
info.get_eof = <file_get_eof_func_ptr>H5FD_fileobj_get_eof
info.read = <file_read_func_ptr>H5FD_fileobj_read
info.write = <file_write_func_ptr>H5FD_fileobj_write
info.truncate = <file_truncate_func_ptr>H5FD_fileobj_truncate
info.flush = <file_flush_func_ptr>H5FD_fileobj_flush
# H5FD_FLMAP_DICHOTOMY
info.fl_map = [H5FD_MEM_SUPER, # default
H5FD_MEM_SUPER, # super
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[build-system]
requires = [
"Cython >=0.29.31,<1",
"Cython >=3",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

"oldest-supported-numpy",
"pkgconfig",
"setuptools >=61",
Expand Down