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.

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

Type Buffer's flags argument #648

Merged
merged 1 commit into from
Nov 18, 2024

Conversation

jakirkham
Copy link
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)

Copy link

codecov bot commented Nov 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.88%. Comparing base (145f57c) to head (c1c95fe).
Report is 4 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #648   +/-   ##
=======================================
  Coverage   99.88%   99.88%           
=======================================
  Files          62       62           
  Lines        2723     2723           
=======================================
  Hits         2720     2720           
  Misses          3        3           
---- 🚨 Try these New Features:

Comment on lines +15 to 16
def __cinit__(self, obj, int flags):
PyObject_GetBuffer(obj, &(self.buffer), flags)
Copy link
Member Author

@jakirkham jakirkham Nov 18, 2024

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
25 of 26 checks passed
@jakirkham jakirkham deleted the typ_buffer_flags branch November 18, 2024 19:18
@jakirkham
Copy link
Member Author

Thanks David! 🙏

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