Type `Buffer`'s `flags` argument by jakirkham · Pull Request #648 · zarr-developers/numcodecs · GitHub
Skip to content

Type Buffer's flags argument#648

Merged
dstansby merged 1 commit into
zarr-developers:mainfrom
jakirkham:typ_buffer_flags
Nov 18, 2024
Merged

Type Buffer's flags argument#648
dstansby merged 1 commit into
zarr-developers:mainfrom
jakirkham:typ_buffer_flags

Conversation

@jakirkham

Copy link
Copy Markdown
Member

Ensure the flags argument in Buffer is correctly typed so it can be passed verbatim to PyObject_GetBuffer. This can also avoid needless conversions to and from Python object type.

TODO:

  • Unit tests and/or doctests in docstrings
  • Tests pass locally
  • Docstrings and API docs for any new/modified user-facing classes and functions
  • Changes documented in docs/release.rst
  • Docs build locally
  • GitHub Actions CI passes
  • Test coverage to 100% (Codecov passes)

@codecov

codecov Bot commented Nov 18, 2024

Copy link
Copy Markdown

Comment thread numcodecs/compat_ext.pyx
Comment on lines +15 to 16
def __cinit__(self, obj, int flags):
PyObject_GetBuffer(obj, &(self.buffer), flags)

@jakirkham jakirkham Nov 18, 2024

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The flags argument here is only used in this PyObject_GetBuffer call. The signature of this function in Cython or in Python is

int PyObject_GetBuffer(object obj, Py_buffer *view, int flags)

Here flags is an int specified by a range of possible constants to provide a specific kind of Py_buffer object

To make sure that flags can be passed through unaltered to this function call, this types the __cinit__ arguments to match

Without this typing, Cython will convert values to Python objects first and then convert them back in this constructor (a needless exercise). This change will fix that issue going forward

@dstansby dstansby merged commit 2309714 into zarr-developers:main Nov 18, 2024
@jakirkham jakirkham deleted the typ_buffer_flags branch November 18, 2024 19:18
@jakirkham

Copy link
Copy Markdown
Member Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants